| Message ID | 20181220152126.18741-1-julien.grall@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm: Allow the user to specify where the RAM is placed in the memory | expand |
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
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,
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
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,
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(-)