| Message ID | 20230717102300.3092062-3-tabba@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Align value generated by get_ram_size() to the host's page size | expand |
On Mon, Jul 17, 2023 at 11:23:00AM +0100, Fuad Tabba wrote: > If host_ram_size() * RAM_SIZE_RATIO does not result in a value > aligned to the host page size, it triggers an error in > __kvm_set_memory_region(), called via the > KVM_SET_USER_MEMORY_REGION ioctl, which requires the size to be > page-aligned. > > Fixes: 18bd8c3bd2a7 ("kvm tools: Don't use all of host RAM for guests by default") > Signed-off-by: Fuad Tabba <tabba@google.com> > --- > builtin-run.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin-run.c b/builtin-run.c > index 2801735..ff8ba0b 100644 > --- a/builtin-run.c > +++ b/builtin-run.c > @@ -406,7 +406,7 @@ static u64 get_ram_size(int nr_cpus) > if (ram_size > available) > ram_size = available; > > - return ram_size; > + return ALIGN(ram_size, host_page_size()); > } I guess we could avoid querying the page size twice if we also factored out a helper to grab _SC_PHYS_PAGES and then did the multiply by RAM_SIZE_RATIO before converting back to bytes. e.g. something like: available = MIN_RAM_SIZE; nrpages = host_ram_nrpages() * RAM_SIZE_RATIO; if (nrpages) available = nrpages * host_page_size(); and then host_ram_size() just calls the two new helpers. What do you think? Will
Hi Will, On Mon, Jul 17, 2023 at 12:03 PM Will Deacon <will@kernel.org> wrote: > > On Mon, Jul 17, 2023 at 11:23:00AM +0100, Fuad Tabba wrote: > > If host_ram_size() * RAM_SIZE_RATIO does not result in a value > > aligned to the host page size, it triggers an error in > > __kvm_set_memory_region(), called via the > > KVM_SET_USER_MEMORY_REGION ioctl, which requires the size to be > > page-aligned. > > > > Fixes: 18bd8c3bd2a7 ("kvm tools: Don't use all of host RAM for guests by default") > > Signed-off-by: Fuad Tabba <tabba@google.com> > > --- > > builtin-run.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/builtin-run.c b/builtin-run.c > > index 2801735..ff8ba0b 100644 > > --- a/builtin-run.c > > +++ b/builtin-run.c > > @@ -406,7 +406,7 @@ static u64 get_ram_size(int nr_cpus) > > if (ram_size > available) > > ram_size = available; > > > > - return ram_size; > > + return ALIGN(ram_size, host_page_size()); > > } > > I guess we could avoid querying the page size twice if we also factored > out a helper to grab _SC_PHYS_PAGES and then did the multiply by > RAM_SIZE_RATIO before converting back to bytes. > > e.g. something like: > > available = MIN_RAM_SIZE; > > nrpages = host_ram_nrpages() * RAM_SIZE_RATIO; > if (nrpages) > available = nrpages * host_page_size(); > > and then host_ram_size() just calls the two new helpers. > > What do you think? Sounds good to me. I'll respin. Cheers, /fuad > > Will
diff --git a/builtin-run.c b/builtin-run.c index 2801735..ff8ba0b 100644 --- a/builtin-run.c +++ b/builtin-run.c @@ -406,7 +406,7 @@ static u64 get_ram_size(int nr_cpus) if (ram_size > available) ram_size = available; - return ram_size; + return ALIGN(ram_size, host_page_size()); } static const char *find_kernel(void)
If host_ram_size() * RAM_SIZE_RATIO does not result in a value aligned to the host page size, it triggers an error in __kvm_set_memory_region(), called via the KVM_SET_USER_MEMORY_REGION ioctl, which requires the size to be page-aligned. Fixes: 18bd8c3bd2a7 ("kvm tools: Don't use all of host RAM for guests by default") Signed-off-by: Fuad Tabba <tabba@google.com> --- builtin-run.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)