> -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: 27 November 2019 15:56 > To: Durrant, Paul <pdurr...@amazon.com>; George Dunlap > <george.dun...@citrix.com> > Cc: xen-devel@lists.xenproject.org; Andrew Cooper > <andrew.coop...@citrix.com>; Anthony PERARD <anthony.per...@citrix.com>; > Roger Pau Monné <roger....@citrix.com>; Volodymyr Babchuk > <volodymyr_babc...@epam.com>; George Dunlap <george.dun...@eu.citrix.com>; > Ian Jackson <ian.jack...@eu.citrix.com>; Marek Marczykowski-Górecki > <marma...@invisiblethingslab.com>; Stefano Stabellini > <sstabell...@kernel.org>; Konrad Rzeszutek Wilk <konrad.w...@oracle.com>; > Julien Grall <jul...@xen.org>; Wei Liu <w...@xen.org> > Subject: Re: [PATCH v2] Rationalize max_grant_frames and > max_maptrack_frames handling > > On 27.11.2019 15:37, Paul Durrant wrote: > > --- a/xen/arch/arm/setup.c > > +++ b/xen/arch/arm/setup.c > > @@ -789,7 +789,7 @@ void __init start_xen(unsigned long > boot_phys_offset, > > .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap, > > .max_evtchn_port = -1, > > .max_grant_frames = gnttab_dom0_frames(), > > - .max_maptrack_frames = opt_max_maptrack_frames, > > + .max_maptrack_frames = -1, > > }; > > int rc; > > > > --- a/xen/arch/x86/setup.c > > +++ b/xen/arch/x86/setup.c > > @@ -697,8 +697,8 @@ void __init noreturn __start_xen(unsigned long > mbi_p) > > struct xen_domctl_createdomain dom0_cfg = { > > .flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity > : 0, > > .max_evtchn_port = -1, > > - .max_grant_frames = opt_max_grant_frames, > > - .max_maptrack_frames = opt_max_maptrack_frames, > > + .max_grant_frames = -1, > > + .max_maptrack_frames = -1, > > }; > > With these there's no need anymore for opt_max_maptrack_frames to > be non-static. Sadly Arm still wants opt_max_grant_frames > accessible in gnttab_dom0_frames(). >
Yes, I was about to make them static until I saw what the ARM code did. > > --- a/xen/common/grant_table.c > > +++ b/xen/common/grant_table.c > > @@ -1837,12 +1837,18 @@ active_alloc_failed: > > return -ENOMEM; > > } > > > > -int grant_table_init(struct domain *d, unsigned int max_grant_frames, > > - unsigned int max_maptrack_frames) > > +int grant_table_init(struct domain *d, int max_grant_frames, > > + int max_maptrack_frames) > > { > > struct grant_table *gt; > > int ret = -ENOMEM; > > > > + /* Default to maximum value if no value was specified */ > > + if ( max_grant_frames < 0 ) > > + max_grant_frames = opt_max_grant_frames; > > + if ( max_maptrack_frames < 0 ) > > + max_maptrack_frames = opt_max_maptrack_frames; > > + > > if ( max_grant_frames < INITIAL_NR_GRANT_FRAMES || > > I take it we don't expect people to specify 2^^31 or more > frames for either option. It looks like almost everything > here would cope, except for this very comparison. Nevertheless > I wonder whether you wouldn't better confine both values to > [0, INT_MAX] now, including when adjusted at runtime. I can certainly remove the 'U' from the definition of INITIAL_NR_GRANT_FRAMES, but do you want me to make opt_max_grant_frames and opt_max_maptrack_frames into signed ints and add signed parser code too? I also don't understand the 'adjusted at runtime' part. Paul > > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel