Re: [Qemu-devel] [PATCH V2 0/5] hw/pvrdma: PVRDMA device implementation

[Yuval Shaia]

On Thu, Dec 21, 2017 at 10:46:35PM +0200, Michael S. Tsirkin wrote:
> On Thu, Dec 21, 2017 at 05:59:38PM +0200, Marcel Apfelbaum wrote:
> > On 21/12/2017 16:22, Michael S. Tsirkin wrote:
> > > On Thu, Dec 21, 2017 at 09:27:51AM +0200, Yuval Shaia wrote:
> > > > > > 
> > > > > > > What happens if guest attempts to register all its memory?
> > > > > > > 
> > > > > > 
> > > > > > Then we loose, is not different from bare metal, reg_mr will pin 
> > > > > > all the RAM.
> > > > > 
> > > > > We need to find a way to communicate to guests about amount
> > > > > of memory they can pin.
> > > > 
> > > > dev_caps.max_mr_size is the way device limits guest driver.
> > > > This value is controlled by the command line argument 
> > > > dev-caps-max-mr-size
> > > > so we should be fine (btw, default value is 1<<32).
> > > 
> > > Isn't that still leaving the option for guest to register all memory,
> > > just in chunks?
> > > 
> > 
> > We also have a parameter limiting the number of mrs (dev-caps-max-mr),
> > together with dev-caps-max-mr-size we can limit the memory the guests can 
> > pin.
> > 
> > Thanks,
> > Marcel
> 
> You might want to limit the default values then.
> 
> Right now:
> 
> +#define MAX_MR_SIZE           (1UL << 32)
> +#define MAX_MR                2048

Maybe limiting by constant number is not a good approach, it looks odd if
one guest with 16G ram and second with 32G ram will have the same settings,
right?
So how about limiting by a specific percentage of total memory?
In that case, what would be this percentage? 100%? 80%?

> 
> Which is IIUC 8TB.
> 
> That's pretty close to unlimited, and so far overcommit seems to be the
> main feature for users.
> 
> 
> > > > > 
> > > > > > However this is only one scenario, and hopefully not much used
> > > > > > for RoCE. (I know IPoIB does that, but it doesn't make sense to use 
> > > > > > it with RoCE).
> > > > > 
> > > > > SRP does it too AFAIK.
> > > > >