Re: [Xen-devel] [PATCH v1 1/2] x86/mm: Add mem access rights to NPT

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.05.18 at 20:30, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 05/18/2018 06:30 PM, Jan Beulich wrote:
>>>>> On 11.05.18 at 13:11, <aisaila@xxxxxxxxxxxxxxx> wrote:
>>> This patch adds access rights for the NPT pages. The access rights are
>>> saved in bits 59:56 of pte that are manipulated through p2m_set_access()
>>> and p2m_get_access() functions.
>> 
>> You don't give any rationale for the choice of bits. Right now p2m-pt.c still
>> assumes that CPU and IOMMU page tables might be shared, despite
>> amd_iommu_init() unconditionally turning this functionality off. As long as 
>> the
>> option for that mode hasn't been removed from p2m-pt.c, I think bits used
>> by the IOMMU (here: bit 59) should not be used for software purposes. The
>> alternative therefore is for you to supply a prereq patch purging the sharing
>> functionality from p2m-pt.c and preferably also from the AMD IOMMU code.
>> That's of course only an option if we don't foresee any means by which this
>> mode may become usable again.
> 
> The choice of bits was our interpretation of Andrew's reply here:
> 
> https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg00573.html 
> 
> Have we misread it?

I don't think you have, but what Andrew has described was only the CPU side
of considerations to make. Plus of course the patch description should explain
whatever choice you make.

> We've also thought about putting the information in a new field of
> struct page_info. Would that perhaps be preferable?

I don't view this as a page property, but a mapping property. As such
it can't validly go into struct page_info.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel