> -----Original Message-----
> From: Jan Beulich <[email protected]>
> Sent: 27 November 2019 15:56
> To: Durrant, Paul <[email protected]>; George Dunlap
> <[email protected]>
> Cc: [email protected]; Andrew Cooper
> <[email protected]>; Anthony PERARD <[email protected]>;
> Roger Pau Monné <[email protected]>; Volodymyr Babchuk
> <[email protected]>; George Dunlap <[email protected]>;
> Ian Jackson <[email protected]>; Marek Marczykowski-Górecki
> <[email protected]>; Stefano Stabellini
> <[email protected]>; Konrad Rzeszutek Wilk <[email protected]>;
> Julien Grall <[email protected]>; Wei Liu <[email protected]>
> 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
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
