[kvmtool,v3,0/9] arm: Allow the user to specify where the RAM is placed in the memory
mbox series

Message ID 20181220152126.18741-1-julien.grall@arm.com
Headers show
Series
  • arm: Allow the user to specify where the RAM is placed in the memory
Related show

Message

Julien Grall Dec. 20, 2018, 3:21 p.m. UTC
Hi all,

At the moment, a user is only able to specify the amount of RAM used by the
guest. Where the RAM will live is left to the software and hardcoded.

It could be useful for testing purpose to move the RAM in different place.
This series adds the possibility for the user to specify multiple RAM region.

The option -m/--mem is extended to specify the address using the following
format: <size>@<addr>. The option needs to be repeated as many times as the
number of RAM region in the guest layout.

For instance, if you want 512MB at 3GB and 512MB 4GB it would look like:
    -m 512@0xc0000000 -m 512@0x100000000

Note that the memory layout is not yet fully configurable by the user, so the
MMIO region is still living below 2GB. This means RAM cannot live in the
region 0-2GB. This could be changed in the future.

Cheers,

Julien Grall (7):
  kvm__arch_init: Don't pass hugetlbfs_path and ram_size in parameter
  virtio/scsi: Allow to use multiple banks
  Fold kvm__init_ram call in kvm__arch_init
  arm: Add an helper to sanitize KVM configuration
  arm: Move anything related to RAM initialization in kvm__init_ram
  Allow the user to specify where the RAM is placed in the memory
  arm: Add support for multi memory regions

Suzuki K Poulose (2):
  arm: Allow use of hugepage with 16K pagesize host
  kvmtool: Allow standard size specifiers for memory bank

 arm/aarch32/include/kvm/kvm-arch.h |   2 +-
 arm/aarch64/include/kvm/kvm-arch.h |   4 +-
 arm/fdt.c                          |  19 +++--
 arm/include/arm-common/kvm-arch.h  |  24 ++++--
 arm/kvm.c                          | 160 ++++++++++++++++++++++++++++++-------
 builtin-run.c                      | 129 +++++++++++++++++++++++++++---
 include/kvm/kvm-config.h           |  16 +++-
 include/kvm/kvm.h                  |  24 ++++--
 kvm.c                              |   8 +-
 mips/kvm.c                         |  47 ++++++-----
 powerpc/kvm.c                      |  33 +++++---
 virtio/scsi.c                      |  21 +++--
 x86/bios.c                         |   8 +-
 x86/kvm.c                          |  54 ++++++++-----
 14 files changed, 417 insertions(+), 132 deletions(-)

Comments

Will Deacon Jan. 22, 2019, 5:50 a.m. UTC | #1
On Thu, Dec 20, 2018 at 03:21:17PM +0000, Julien Grall wrote:
> At the moment, a user is only able to specify the amount of RAM used by the
> guest. Where the RAM will live is left to the software and hardcoded.
> 
> It could be useful for testing purpose to move the RAM in different place.
> This series adds the possibility for the user to specify multiple RAM region.
> 
> The option -m/--mem is extended to specify the address using the following
> format: <size>@<addr>. The option needs to be repeated as many times as the
> number of RAM region in the guest layout.
> 
> For instance, if you want 512MB at 3GB and 512MB 4GB it would look like:
>     -m 512@0xc0000000 -m 512@0x100000000
> 
> Note that the memory layout is not yet fully configurable by the user, so the
> MMIO region is still living below 2GB. This means RAM cannot live in the
> region 0-2GB. This could be changed in the future.

Although the implementation mostly looks ok (I've commented on some things I
spotted), I'm not completely sold on the need for this. What sort of testing
do you have in mind that requires multiple memory banks to be specified at
runtime? I'm keen to keep kvmtool as simple and lean as we can, whilst
supporting enough functionality to be useful as a prototyping tool for the
latest KVM features. I'm just struggling to see how support for multiple
memory banks really fits in with that, and whether or not we can't just make
it easier to configure this at compile time instead.

Finally, your series also seems to duplicate the memory bank information in
three places:

	* struct kvm.ram[]
	* struct kvm.mem_banks
	* struct kvm_config.ram[]

and these aren't used consistently between functions. If we do go ahead with
this, I think this needs consolidating so that e.g. the mem_banks are
authoritative (but I appreciate that you've just extended what we had
already).

Will
Julien Grall Feb. 19, 2019, 6:42 p.m. UTC | #2
Hi Will,

On 1/22/19 5:50 AM, Will Deacon wrote:
> On Thu, Dec 20, 2018 at 03:21:17PM +0000, Julien Grall wrote:
>> At the moment, a user is only able to specify the amount of RAM used by the
>> guest. Where the RAM will live is left to the software and hardcoded.
>>
>> It could be useful for testing purpose to move the RAM in different place.
>> This series adds the possibility for the user to specify multiple RAM region.
>>
>> The option -m/--mem is extended to specify the address using the following
>> format: <size>@<addr>. The option needs to be repeated as many times as the
>> number of RAM region in the guest layout.
>>
>> For instance, if you want 512MB at 3GB and 512MB 4GB it would look like:
>>      -m 512@0xc0000000 -m 512@0x100000000
>>
>> Note that the memory layout is not yet fully configurable by the user, so the
>> MMIO region is still living below 2GB. This means RAM cannot live in the
>> region 0-2GB. This could be changed in the future.
> 
> Although the implementation mostly looks ok (I've commented on some things I
> spotted), I'm not completely sold on the need for this. What sort of testing
> do you have in mind that requires multiple memory banks to be specified at
> runtime? I'm keen to keep kvmtool as simple and lean as we can, whilst
> supporting enough functionality to be useful as a prototyping tool for the
> latest KVM features. I'm just struggling to see how support for multiple
> memory banks really fits in with that, and whether or not we can't just make
> it easier to configure this at compile time instead.
This series turns kvmtool to a useful tool for exercising booting a 
kernel with different memory layout. You can imagine use it when 
touching the memory code (i.e 52-bit...) or even as a testsuite.

For this use case, the compile time option would make things a bit more 
difficult. Hence the runtime option.

> 
> Finally, your series also seems to duplicate the memory bank information in
> three places:
> 
> 	* struct kvm.ram[]
> 	* struct kvm.mem_banks
> 	* struct kvm_config.ram[]
> 
> and these aren't used consistently between functions. If we do go ahead with
> this, I think this needs consolidating so that e.g. the mem_banks are
> authoritative (but I appreciate that you've just extended what we had
> already).

I can have a look at it once we agreed on the way to go ahead.

Cheers,
Will Deacon Feb. 20, 2019, 2:47 p.m. UTC | #3
On Tue, Feb 19, 2019 at 06:42:08PM +0000, Julien Grall wrote:
> On 1/22/19 5:50 AM, Will Deacon wrote:
> > On Thu, Dec 20, 2018 at 03:21:17PM +0000, Julien Grall wrote:
> > > At the moment, a user is only able to specify the amount of RAM used by the
> > > guest. Where the RAM will live is left to the software and hardcoded.
> > > 
> > > It could be useful for testing purpose to move the RAM in different place.
> > > This series adds the possibility for the user to specify multiple RAM region.
> > > 
> > > The option -m/--mem is extended to specify the address using the following
> > > format: <size>@<addr>. The option needs to be repeated as many times as the
> > > number of RAM region in the guest layout.
> > > 
> > > For instance, if you want 512MB at 3GB and 512MB 4GB it would look like:
> > >      -m 512@0xc0000000 -m 512@0x100000000
> > > 
> > > Note that the memory layout is not yet fully configurable by the user, so the
> > > MMIO region is still living below 2GB. This means RAM cannot live in the
> > > region 0-2GB. This could be changed in the future.
> > 
> > Although the implementation mostly looks ok (I've commented on some things I
> > spotted), I'm not completely sold on the need for this. What sort of testing
> > do you have in mind that requires multiple memory banks to be specified at
> > runtime? I'm keen to keep kvmtool as simple and lean as we can, whilst
> > supporting enough functionality to be useful as a prototyping tool for the
> > latest KVM features. I'm just struggling to see how support for multiple
> > memory banks really fits in with that, and whether or not we can't just make
> > it easier to configure this at compile time instead.
> This series turns kvmtool to a useful tool for exercising booting a kernel
> with different memory layout. You can imagine use it when touching the
> memory code (i.e 52-bit...) or even as a testsuite.
> 
> For this use case, the compile time option would make things a bit more
> difficult. Hence the runtime option.

Ok, fair enough. I spoke to Julien Thierry for a second opinion on this, and
perhaps a way forward would be to restrict ourselves to a single memory
bank, but allow it's base address to be provided on the command line? I
think that covers your use-cases above, and should avoid a fair amount of
complication.

What do you reckon?

Will
Julien Grall March 8, 2019, 2:10 p.m. UTC | #4
Hi Will,

On 2/20/19 2:47 PM, Will Deacon wrote:
> On Tue, Feb 19, 2019 at 06:42:08PM +0000, Julien Grall wrote:
>> On 1/22/19 5:50 AM, Will Deacon wrote:
>>> On Thu, Dec 20, 2018 at 03:21:17PM +0000, Julien Grall wrote:
>>>> At the moment, a user is only able to specify the amount of RAM used by the
>>>> guest. Where the RAM will live is left to the software and hardcoded.
>>>>
>>>> It could be useful for testing purpose to move the RAM in different place.
>>>> This series adds the possibility for the user to specify multiple RAM region.
>>>>
>>>> The option -m/--mem is extended to specify the address using the following
>>>> format: <size>@<addr>. The option needs to be repeated as many times as the
>>>> number of RAM region in the guest layout.
>>>>
>>>> For instance, if you want 512MB at 3GB and 512MB 4GB it would look like:
>>>>       -m 512@0xc0000000 -m 512@0x100000000
>>>>
>>>> Note that the memory layout is not yet fully configurable by the user, so the
>>>> MMIO region is still living below 2GB. This means RAM cannot live in the
>>>> region 0-2GB. This could be changed in the future.
>>>
>>> Although the implementation mostly looks ok (I've commented on some things I
>>> spotted), I'm not completely sold on the need for this. What sort of testing
>>> do you have in mind that requires multiple memory banks to be specified at
>>> runtime? I'm keen to keep kvmtool as simple and lean as we can, whilst
>>> supporting enough functionality to be useful as a prototyping tool for the
>>> latest KVM features. I'm just struggling to see how support for multiple
>>> memory banks really fits in with that, and whether or not we can't just make
>>> it easier to configure this at compile time instead.
>> This series turns kvmtool to a useful tool for exercising booting a kernel
>> with different memory layout. You can imagine use it when touching the
>> memory code (i.e 52-bit...) or even as a testsuite.
>>
>> For this use case, the compile time option would make things a bit more
>> difficult. Hence the runtime option.
> 
> Ok, fair enough. I spoke to Julien Thierry for a second opinion on this, and
> perhaps a way forward would be to restrict ourselves to a single memory
> bank, but allow it's base address to be provided on the command line? I
> think that covers your use-cases above, and should avoid a fair amount of
> complication.

This would be a good start, although I suspect multibanks would still be 
useful in the future. We can revisit it later on.

> 
> What do you reckon?

I will have a look.

Cheers,