Re: [Xen-devel] [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use typesafe gfn

[Julien Grall <julien@xxxxxxx>]

Hi Paul,

On 23/03/2020 08:37, Paul Durrant wrote:
> > -----Original Message-----
> > From: julien@xxxxxxx <julien@xxxxxxx>
> > Sent: 22 March 2020 16:14
> > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: julien@xxxxxxx; Julien Grall <julien.grall@xxxxxxx>; Stefano Stabellini 
> > <sstabellini@xxxxxxxxxx>;
> > Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; Andrew Cooper 
> > <andrew.cooper3@xxxxxxxxxx>; George
> > Dunlap <george.dunlap@xxxxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; Jan 
> > Beulich
> > <jbeulich@xxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné 
> > <roger.pau@xxxxxxxxxx>; Paul Durrant
> > <paul@xxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin Tian 
> > <kevin.tian@xxxxxxxxx>; Tim Deegan
> > <tim@xxxxxxx>
> > Subject: [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use 
> > typesafe gfn
> >
> > From: Julien Grall <julien.grall@xxxxxxx>
> >
> > 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 <julien.grall@xxxxxxx>
> Definitely an improvement so...
>
> Reviewed-by: Paul Durrant <paul@xxxxxxx>
>
> But a couple of things I noticed...
>
> [snip]
> > diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
> > index 5d5a746a25..3c29ff86be 100644
> > --- a/xen/arch/x86/hvm/domain.c
> > +++ b/xen/arch/x86/hvm/domain.c
> > @@ -296,8 +296,10 @@ int arch_set_info_hvm_guest(struct vcpu *v, const 
> > vcpu_hvm_context_t *ctx)
> >       if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
> >       {
> >           /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
> > -        struct page_info *page = get_page_from_gfn(v->domain,
> > -                                 v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
> > +        struct page_info *page;
> > +
> > +        page = get_page_from_gfn(v->domain,
> > +                                 gaddr_to_gfn(v->arch.hvm.guest_cr[3]),
> Should this be cr3_to_gfn?
Definitely yes. I thought I spotted all the use when introducing the new 
helper but it looks like not. I will update the patch in the new version 
to use cr3_to_gfn() everywhere you suggested.
Thanks.

Cheers,

--
Julien Grall

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