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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 19.07.18 at 10:43, <rcojocaru@bitdefender.com> wrote:
> On 07/19/2018 11:30 AM, Jan Beulich wrote:
>>>>> On 19.07.18 at 10:18, <aisaila@bitdefender.com> 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.

All fine, but George's question was raised in the context of permission
conversion from p2m to pte representation.

Jan


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