On Fri, Feb 7, 2020 at 10:16 AM Tamas K Lengyel <ta...@tklengyel.com> wrote: > > On Fri, Feb 7, 2020 at 9:54 AM Jan Beulich <jbeul...@suse.com> wrote: > > > > On 07.02.2020 10:52, Roger Pau Monné wrote: > > > On Fri, Feb 07, 2020 at 09:08:15AM +0100, Jan Beulich wrote: > > >> On 06.02.2020 16:20, Jan Beulich wrote: > > >>> --- a/xen/arch/x86/hvm/vmx/vmx.c > > >>> +++ b/xen/arch/x86/hvm/vmx/vmx.c > > >>> @@ -3037,9 +3037,8 @@ static int vmx_alloc_vlapic_mapping(stru > > >>> share_xen_page_with_guest(pg, d, SHARE_rw); > > >>> d->arch.hvm.vmx.apic_access_mfn = mfn; > > >>> > > >>> - return set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE), > > >>> mfn, > > >>> - PAGE_ORDER_4K, > > >>> - p2m_get_hostp2m(d)->default_access); > > >>> + return set_mmio_p2m_entry(d, gaddr_to_gfn(APIC_DEFAULT_PHYS_BASE), > > >>> mfn, > > >>> + PAGE_ORDER_4K); > > >>> } > > >> > > >> Upon 2nd thought - does this really want to use default access? > > >> Execute permission for this page looks a little suspicious. > > >> Isn't it the case that this page doesn't (normally?) get > > >> accessed at all, and instead its address serves as an indicator > > >> to the CPU? (I even vaguely recall it having been considered to > > >> consolidate this, to e.g. a single page per domain.) In which > > >> case even p2m_access_n might be good enough? > > > > > > Hm, I think p2m_access_n is not enough, as that would trigger an EPT > > > fault which has preference over the APIC access VM exit (see 29.4.1 > > > Priority of APIC-Access VM Exits). > > > > Ah, yes, reading that text I agree. Having just a single such page > > per domain would still seem possible, though. Although, if we meant > > to support a guest moving the APIC base address, then we couldn't, > > again. > > > > > I think setting it to p2m_access_rw should be enough, and we would get > > > EPT faults when trying to execute from APIC page. > > > > Then the other question is whether there's any use for introspection > > to further limit permissions on this (kind of fake) page. Tamas? > > I'm not aware of any use-case that would restrict the EPT permission > for MMIO pages. That said, an application could still request that to > be set later on. Since this function here gets called in > vmx_domain_initialise I suspect a mem_access user didn't even have a > chance to change the default_access to anything custom so I venture it > would be safe to choose whatever permission is sensible. If anyone > wants to mess with the permission later they can do that regardless of > what was set here.
One thing to add though regarding using p2m_access_rw here is that in case something would trigger an X violation it would lead to an event being sent to a vm_event subscriber, which they may not be able to make sense of. So I would suggest that if you want to make this pagetable entry R/W only to use a p2m_type for that instead of a p2m_access. Tamas _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
