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

[George Dunlap <george.dunlap@citrix.com>]

On 6/13/19 11:56 AM, Alexandru Stefan ISAILA wrote:
> 
> 
> On 26.09.2018 19:47, George Dunlap wrote:
>> From: Isaila Alexandru <aisaila@bitdefender.com>
>>
>> This patch adds access control for NPT mode.
>>
>> There aren’t enough extra bits to store the access rights in the NPT p2m
>> table, so we add a radix tree to store extra information.
>>
>> For efficiency:
>>   - Only allocate this radix tree when we first store "non-default"
>>     extra information
>>
>>   - Remove entires which match the default extra information rather
>>     than continuing to store them
>>
>>   - For superpages, only store an entry for the first gfn in the
>>     superpage.  Use the order of the p2m entry being read to determine
>>     the proper place to look in the radix table.
>>
>> Modify p2m_type_to_flags() to accept and interpret an access value,
>> parallel to the ept code.
>>
>> Add a set_default_access() method to the p2m-pt and p2m-ept versions
>> of the p2m rather than setting it directly, to deal with different
>> default permitted access values.
>>
>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
>> ---
>> NB, this is compile-tested only.
>>
>> cc'ing Paul because this is functionality he may want at some point in
>> the future.
>>
>> I'm not sure why we only allow 'int' to be stored in the radix tree,
>> but that throws away 30-some bits we could otherwise use.  We might
>> consider revising this if we run out of bits here.
>>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Tim Deegan <tim@xen.org>
>> CC: Tamas K Lengyel <tamas.lengyel@zentific.com>
>> CC: Paul Durrant <paul.durrant@citrix.com>
>> CC: Razvan Cojocaru <rcojocaru@bitdefender.com>
> 
> Hi all,
> 
> I know it's been some time from the start of this patch but can this 
> move forward? Any thoughts or acks are appreciated.

Right, well where we left this, the situation was that on AMD hardware
with IOMMU / p2m sharing, there aren't enough bits.

The two general fixes we have so far are:
1. Add a parallel tree for extra bits (this patch)
2. Rip out IOMMU / p2m sharing for AMD.

#2 has the advantage that we don't need an entirely separate tree, as
well as getting rid of code that has (apparently) been completely dead
for 5 years. #1 has the advantage that we're set up for having a much
larger number of IOREQ servers in the future.

Nobody objected to #2.  Without looking deeply into it, it seems like it
might be a good idea, but I can't be sure without seeing what it would
actually look like.

The easiest way to press the point then would be to post a patch series
ripping it out.

 -George

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