Re: [Xen-devel] [PATCH 1/1 V3] x86/AMD: Fix nested svm crash due to assertion in __virt_to_maddr

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.07.13 at 23:17, Suravee Suthikulanit <suravee.suthikulpanit@xxxxxxx>
wrote:
> On 7/12/2013 3:01 AM, Jan Beulich wrote:
>>>>> On 11.07.13 at 19:34, <suravee.suthikulpanit@xxxxxxx> wrote:
>>> --- a/xen/arch/x86/mm/hap/nested_hap.c
>>> +++ b/xen/arch/x86/mm/hap/nested_hap.c
>>> @@ -191,6 +191,19 @@ out:
>>>       return rc;
>>>   }
>>>   
>>> +int
>>> +nestedhvm_walk_L0_p2m(struct vcpu *v, paddr_t L1_gpa, paddr_t *L0_gpa)
>>> +{
>>> +    p2m_type_t p2mt_10;
>>> +    unsigned int page_order_10;
>>> +    p2m_access_t p2ma_10 = p2m_access_rwx;
>> Pointless initializer?
> These are mostly part of the required function prototype.  However, I 
> noticed that I don't need to specify page order.

I don't follow - how is p2ma_10 getting (pointlessly afaict)
initialized related to the prototype of the function?

>>> +                                 L1_gpa, L0_gpa,
>>> +                                 &p2mt_10, &p2ma_10, &page_order_10,
>>> +                                 0, 0, 0);
>> Wouldn't the caller's use of the GPA imply that you want read and
>> write access here?
> Actually, access_r and access_x is not used in the 
> "nestedhap_walk_L0_p2m" function.
> Since we are not writing to this GPA, would we need the write access?

I may be misunderstanding this (and would hope for Tim to correct
me if so), but generally page table walks get done with the goal of
using the translation for a specific kind of access(es). And the
types of these accesses ought to determine what permissions you
want to be checked for during the walk. In the case here you want
the VMCB to be read from and written to, so you'd want to ask for
read and write permission.

Jan


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