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

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 07/19/2018 11:30 AM, Jan Beulich wrote:
>>>> On 19.07.18 at 10:18, <aisaila@xxxxxxxxxxxxxxx> wrote:
>> On Mi, 2018-07-18 at 15:33 +0000, George Dunlap wrote:
>>>> On Jul 2, 2018, at 8:42 AM, Alexandru Isaila <aisaila@bitdefender.c 
>>>> +            break;
>>>> +        case p2m_access_x:
>>>> +            flags &= ~_PAGE_RW;
>>>> +            break;
>>>> +        case p2m_access_rwx:
>>>> +        default:
>>>> +            break;
>>>>     }
>>> I think you want another blank line here too.
>>>
>>> Also, this doesn’t seem to capture the ‘r’ part of the equation —
>>> shouldn’t p2m_access_n end up with a not-present p2m entry?
>>
>> SVM dosen't explicitly provide a read access bit so we treat read and
>> write the same way.
> 
> Read and write can't possibly be treated the same. You ought to use
> the present bit to deny read (really: any) access, as also implied by
> George's response.

They aren't treated the same as far sending out a vm_event goes.
However, if we understand this correctly, there is no way to cause only
read, or only write exits for NPT. They are bundled together under _PAGE_RW.

So svm_do_nested_pgfault() tries to sort these out:

1781     struct npfec npfec = {
1782         .read_access = !(pfec & PFEC_insn_fetch),
1783         .write_access = !!(pfec & PFEC_write_access),
1784         .insn_fetch = !!(pfec & PFEC_insn_fetch),
1785         .present = !!(pfec & PFEC_page_present),
1786     };
1787
1788     /* These bits are mutually exclusive */
1789     if ( pfec & NPT_PFEC_with_gla )
1790         npfec.kind = npfec_kind_with_gla;
1791     else if ( pfec & NPT_PFEC_in_gpt )
1792         npfec.kind = npfec_kind_in_gpt;
1793
1794     ret = hvm_hap_nested_page_fault(gpa, ~0ul, npfec);

but a read access is considered to be something that's not an insn
fetch, and we only have a specific bit set for the write.

Since hvm_hap_nested_page_fault() looks at npfec to decide when to send
out a vm_event, this takes care of handling reads and writes differently
at this level; however it's not possible to set separate single "don't
read" or "don't write" exit-causing flags with NPT.


Thanks,
Razvan

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