[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [V3 PATCH 7/9] x86/hvm: pkeys, add pkeys support for guest_walk_tables



On 16/12/15 16:28, Tim Deegan wrote:
> Hi,
>
> At 15:36 +0000 on 16 Dec (1450280191), George Dunlap wrote:
>> (hvm_fetch_from_guest_virt() seems to only set PFEC_insn_fetch if nx or
>> smep are enabled in the guest.  This seems inconsistent to me with the
>> treatment of PFEC_reserved_bit: it seems like
>> hvm_fetch_from_guest_virt() should always pass in PFEC_insn_fetch,
>> particularly as guest_walk_tables() will already gate the checks based
>> on whether nx or smep is enabled in the guest.  Tim, you know of any
>> reason for this?)
> This code is following the hardware, IIRC: on real CPUs without NX
> enabled, pagefault error codes will not have that bit set even when
> caused by instruction fetches.

There is a difference between Intel and AMD.  Intel has the bit reserved
(and therefore faults), while AMD declares the bit as ignored (and
doesn't fault).

These are yet more subtle differences which are not quite accounted for
correctly in Xen (which I am frankly unlikely to even get to in cpuid v2).

~Andrew

>   Looking at the SDM (3A.4.7) the
> current rules are for that bit to be set iff ((PAE && NXE) || SMEP).
>
> It seems unfortunate to have this check duplicated between
> guest_walk_tables() and its callers, so potentially g_w_t() should
> explicitly _clear_ PFEC_insn_fetch in cases where it should not be
> returned, and then its callers can set it unconditionally for
> all instruction fetches.
>
> (There are similar rules for the new PK bit, which should not be shown
> to the guest if CR4.PKE is clear.)
>
> Cheers,
>
> Tim.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.