[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v5 2/3] x86/altp2m: Add a hvmop for setting the suppress #VE bit
On 9/23/18 7:33 PM, George Dunlap wrote: > Sorry, looks like this may not have gone through. > -G > On Thu, Sep 20, 2018 at 5:08 PM George Dunlap <george.dunlap@xxxxxxxxxx> > wrote: >> >> On Thu, Sep 20, 2018 at 4:53 PM Razvan Cojocaru >> <rcojocaru@xxxxxxxxxxxxxxx> wrote: >>> >>> On 9/20/18 2:34 PM, George Dunlap wrote: >>>>> +int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve, >>>>> + unsigned int altp2m_idx) >>>> This should clearly be in p2m.c, and... >>>> >>>>> +{ >>>>> + struct p2m_domain *host_p2m = p2m_get_hostp2m(d); >>>>> + struct p2m_domain *ap2m = NULL; >>>>> + struct p2m_domain *p2m; >>>>> + mfn_t mfn; >>>>> + p2m_access_t a; >>>>> + p2m_type_t t; >>>>> + int rc; >>>>> + >>>>> + if ( !cpu_has_vmx_virt_exceptions ) >>>>> + return -EOPNOTSUPP; >>>> We should avoid Intel-specific checks in common code. >>>> >>>> In fact, this is wrong, because you can choose to run a guest in shadow >>>> mode even on a system with virt exceptions -- in which case you'd >>>> trigger the ASSERT() in p2m-pt.c:p2m_pt_set_entry(). >>>> >>>> Probably what should happen is that we should move the vmx check into >>>> p2m-ept.c:p2m_ept_set_entry(), and replace the ASSERT(sve = 0) in >>>> p2m-pt.c:p2m_pt_set_entry() with "if ( sve != 0 ) return -ENOTSUPP;". >>>> >>>> Although what should probably *really* happen is that >>>> `HVMOP_altp2m_vcpu_enable_notify` should fail with -EOPNOTSUPP instead >>>> of silently succeeding. >>> >>> Do you mean HVMOP_altp2m_set_suppress_ve here, or am I misunderstanding >>> your comment? I'm happy to do the exact modifications you've requested >>> above but I'm afraid I don't follow the HVMOP_altp2m_vcpu_enable_notify >>> comment. >> >> At the moment, it appears that you can call >> HVMOP_altp2m_vcpu_enable_notify to set a vcpu's veinfo_gfn even if the >> hardware or the p2m mode doesn't support #VE. It would be better if >> callers got the -EOPNOTSUPP error at that point, rather than having >> vcpu_enable_notify succeed and then having set_suppress_ve fail. >> >> To be clear, I'm not requiring that as a clean-up, I'm just saying >> that such a change would be an improvement (which you may consider >> doing if you wish). Undestood, thank you for the clarification! We'll follow up with a patch on that. I have in the meantime sent out a patch with the requested altp2m cleanup (code moved to p2m.c and checks moved to p2m-ept.c. I haven't seen a reply to my question about whether this is what you meant (perhaps it got lost as well?), so I hope I've understood your request correctly. Thanks, Razvan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |