Re: [PATCH 3/3] hw/arm/virt: Implement kvm-steal-time

[Andrew Jones]

On Sat, Aug 01, 2020 at 02:00:34PM +0200, Andrew Jones wrote:
> > >      if (kvm_enabled()) {
> > > +        hwaddr pvtime_base = vms->memmap[VIRT_PVTIME].base;
> > > +        hwaddr pvtime_size = vms->memmap[VIRT_PVTIME].size;
> > > +
> > > +        if (steal_time) {
> > > +            MemoryRegion *pvtime = g_new(MemoryRegion, 1);
> > > +
> > > +            memory_region_init_ram(pvtime, NULL, "pvtime", pvtime_size, 
> > > NULL);
> > > +            memory_region_add_subregion(get_system_memory(), pvtime_base,
> > > +                                        pvtime);
> > > +        }
> > 
> > B: I'm not sure whether it wouldn't be useful to have the area
> > allocated with size determined by number of VCPUs instead of having
> > pre-defined size.
> 
> We can't go smaller than one host-sized page, so this 64k region is the
> smallest we can go. The assert in the next hunk, which was snipped
> out of the reply, ensures that 64k is large enough to cover the maximum
> number of VCPUs that could ever be configured. I don't think there's
> anything else we should do at this time. If the pvtime structure grows,
> or if we increase the maximum number of VCPUs to be larger than 1024,
> then we can revisit this in order to determine when additional 64k pages
> should be allocated.
> 
> For now, if it would help, I could extend the comment (which was also
> snipped) to mention that 64k was chosen because it's the maximum host
> page size, and that at least one host-sized page must be allocated for
> this region.
>

In the end, I think I will calculate the size based on host-page-size
and the number of possible guest cpus. The main reason is that it's
actually more awkward to properly document it in a comment than to just
code it. And, we'll also reduce the number of pages that would need to
be migrated when the host is using 4k pages.

Thanks,
drew