Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn

From: Julien Grall <julien.grall@arm.com>
To: Paul Durrant <Paul.Durrant@citrix.com>,
	"sstabellini@kernel.org" <sstabellini@kernel.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Kevin Tian <kevin.tian@intel.com>, Wei Liu <wei.liu2@citrix.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Andrew Cooper <Andrew.Cooper3@citrix.com>,
	"Tim (Xen.org)" <tim@xen.org>,
	George Dunlap <George.Dunlap@citrix.com>,
	Julien Grall <julie.grall@arm.com>,
	Jan Beulich <jbeulich@suse.com>,
	Ian Jackson <Ian.Jackson@citrix.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Brian Woods <brian.woods@amd.com>
Subject: Re: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use typesafe gfn
Date: Wed, 7 Nov 2018 13:05:52 +0000	[thread overview]
Message-ID: <33905da4-8deb-14a9-a7af-a94e4d19b42a@arm.com> (raw)
In-Reply-To: <45f3cb5c91af4f2b801b78e07bcc5164@AMSPEX02CL03.citrite.net>

Hi Paul,

On 07/11/2018 09:24, Paul Durrant wrote:
>> -----Original Message-----
>> From: Julien Grall [mailto:julien.grall@arm.com]
>> Sent: 06 November 2018 19:15
>> To: sstabellini@kernel.org; xen-devel@lists.xenproject.org
>> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper
>> <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Ian
>> Jackson <Ian.Jackson@citrix.com>; Jan Beulich <jbeulich@suse.com>; Konrad
>> Rzeszutek Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org>; Wei
>> Liu <wei.liu2@citrix.com>; Boris Ostrovsky <boris.ostrovsky@oracle.com>;
>> Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Brian Woods
>> <brian.woods@amd.com>; Paul Durrant <Paul.Durrant@citrix.com>; Jun
>> Nakajima <jun.nakajima@intel.com>; Kevin Tian <kevin.tian@intel.com>;
>> Julien Grall <julie.grall@arm.com>
>> Subject: [PATCH 8/8] xen: Swich parameter in get_page_from_gfn to use
>> typesafe gfn
>>
>> No functional change intended.
>>
>> Only reasonable clean-ups are done in this patch. The rest will use _gfn
>> for the time being.
>>
>> Signed-off-by: Julien Grall <julie.grall@arm.com>
>> ---
>>   xen/arch/arm/guestcopy.c             |  2 +-
>>   xen/arch/arm/mm.c                    |  2 +-
>>   xen/arch/x86/cpu/vpmu.c              |  2 +-
>>   xen/arch/x86/domain.c                | 12 ++++++------
>>   xen/arch/x86/domctl.c                |  6 +++---
>>   xen/arch/x86/hvm/dm.c                |  2 +-
>>   xen/arch/x86/hvm/domain.c            |  2 +-
>>   xen/arch/x86/hvm/hvm.c               |  9 +++++----
>>   xen/arch/x86/hvm/svm/svm.c           |  8 ++++----
>>   xen/arch/x86/hvm/viridian/viridian.c | 24 ++++++++++++------------
>>   xen/arch/x86/hvm/vmx/vmx.c           |  4 ++--
>>   xen/arch/x86/hvm/vmx/vvmx.c          | 12 ++++++------
>>   xen/arch/x86/mm.c                    | 24 ++++++++++++++----------
>>   xen/arch/x86/mm/p2m.c                |  2 +-
>>   xen/arch/x86/mm/shadow/hvm.c         |  6 +++---
>>   xen/arch/x86/physdev.c               |  3 ++-
>>   xen/arch/x86/pv/descriptor-tables.c  |  5 ++---
>>   xen/arch/x86/pv/emul-priv-op.c       |  6 +++---
>>   xen/arch/x86/pv/mm.c                 |  2 +-
>>   xen/arch/x86/traps.c                 | 11 ++++++-----
>>   xen/common/domain.c                  |  2 +-
>>   xen/common/event_fifo.c              | 12 ++++++------
>>   xen/common/memory.c                  |  4 ++--
>>   xen/common/tmem_xen.c                |  2 +-
>>   xen/include/asm-arm/p2m.h            |  6 +++---
>>   xen/include/asm-x86/p2m.h            | 11 +++++++----
>>   26 files changed, 95 insertions(+), 86 deletions(-)
>>
> [snip]
>> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
>> index 5d00256aaa..a7419bd444 100644
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -317,7 +317,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct
>> hvm_hw_cpu *c)
>>       {
>>           if ( c->cr0 & X86_CR0_PG )
>>           {
>> -            page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT,
>> +            page = get_page_from_gfn(v->domain, gaddr_to_gfn(c->cr3),
>>                                        NULL, P2M_ALLOC);
>>               if ( !page )
>>               {
>> @@ -2412,9 +2412,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t
>> vmcbaddr)
>>           return NULL;
>>
>>       /* Need to translate L1-GPA to MPA */
>> -    page = get_page_from_gfn(v->domain,
>> -                            nv->nv_vvmcxaddr >> PAGE_SHIFT,
>> -                            &p2mt, P2M_ALLOC | P2M_UNSHARE);
>> +    page = get_page_from_gfn(v->domain,
>> +                             gaddr_to_gfn(nv->nv_vvmcxaddr >>
>> PAGE_SHIFT),
> 
> Don't you need to lose the '>> PAGE_SHIFT' now?

Yes. Brian reported on IRC and now it is fixed.

Thank you for the review.

Cheers,

-- 
Julien Grall

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