| Message ID | 20220629103905.24480-1-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvmtool] arm64: pvtime: Use correct region size | expand |
On Wed, 29 Jun 2022 11:39:05 +0100 Alexandru Elisei <alexandru.elisei@arm.com> wrote: Hi, > pvtime uses ARM_PVTIME_BASE instead of ARM_PVTIME_SIZE for the size of the > memory region given to the guest, which causes to the following error when > creating a flash device (via the -F/--flash command line argument): > > Error: RAM (read-only) region [2000000-27fffff] would overlap RAM region [1020000-203ffff] > > The read-only region represents the guest memory where the flash image is > copied by kvmtool. The region starting at 0x102_0000 (ARM_PVTIME_BASE) is > the pvtime region, which should be 64K in size. kvmtool erroneously creates > the region to be ARM_PVTIME_BASE in size instead, and the last address > becomes: > > ARM_PVTIME_BASE + ARM_PVTIME_BASE - 1 = 0x102_0000 + 0x102_0000 - 1 = 0x203_ffff > > which corresponds to the end of the region from the error message. > > Do the right thing and make the pvtime memory region ARM_PVTIME_SIZE = 64K > bytes, as it was intended. > > Fixes: 7d4671e5d372 ("aarch64: Add stolen time support") > Reported-by: Pierre Gondois <pierre.gondois@arm.com> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> Oops, this looks indeed like a glaring bug, PVTIME_SIZE was not used anywhere. Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > --- > arm/aarch64/pvtime.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/arm/aarch64/pvtime.c b/arm/aarch64/pvtime.c > index a49cf3ed5478..2933ac7ca959 100644 > --- a/arm/aarch64/pvtime.c > +++ b/arm/aarch64/pvtime.c > @@ -14,15 +14,15 @@ static int pvtime__alloc_region(struct kvm *kvm) > char *mem; > int ret = 0; > > - mem = mmap(NULL, ARM_PVTIME_BASE, PROT_RW, > + mem = mmap(NULL, ARM_PVTIME_SIZE, PROT_RW, > MAP_ANON_NORESERVE, -1, 0); > if (mem == MAP_FAILED) > return -errno; > > ret = kvm__register_ram(kvm, ARM_PVTIME_BASE, > - ARM_PVTIME_BASE, mem); > + ARM_PVTIME_SIZE, mem); > if (ret) { > - munmap(mem, ARM_PVTIME_BASE); > + munmap(mem, ARM_PVTIME_SIZE); > return ret; > } > > @@ -36,8 +36,8 @@ static int pvtime__teardown_region(struct kvm *kvm) > return 0; > > kvm__destroy_mem(kvm, ARM_PVTIME_BASE, > - ARM_PVTIME_BASE, usr_mem); > - munmap(usr_mem, ARM_PVTIME_BASE); > + ARM_PVTIME_SIZE, usr_mem); > + munmap(usr_mem, ARM_PVTIME_SIZE); > usr_mem = NULL; > return 0; > }
On Wed, Jun 29, 2022 at 11:39:05AM +0100, Alexandru Elisei wrote: > pvtime uses ARM_PVTIME_BASE instead of ARM_PVTIME_SIZE for the size of the > memory region given to the guest, which causes to the following error when > creating a flash device (via the -F/--flash command line argument): > > Error: RAM (read-only) region [2000000-27fffff] would overlap RAM region [1020000-203ffff] > > The read-only region represents the guest memory where the flash image is > copied by kvmtool. The region starting at 0x102_0000 (ARM_PVTIME_BASE) is > the pvtime region, which should be 64K in size. kvmtool erroneously creates > the region to be ARM_PVTIME_BASE in size instead, and the last address > becomes: > > ARM_PVTIME_BASE + ARM_PVTIME_BASE - 1 = 0x102_0000 + 0x102_0000 - 1 = 0x203_ffff > > which corresponds to the end of the region from the error message. > > Do the right thing and make the pvtime memory region ARM_PVTIME_SIZE = 64K > bytes, as it was intended. > > Fixes: 7d4671e5d372 ("aarch64: Add stolen time support") > Reported-by: Pierre Gondois <pierre.gondois@arm.com> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > --- > arm/aarch64/pvtime.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > Hi, Thanks for fixing this. Reviewed-by: Sebastian Ene <sebastianene@google.com> > diff --git a/arm/aarch64/pvtime.c b/arm/aarch64/pvtime.c > index a49cf3ed5478..2933ac7ca959 100644 > --- a/arm/aarch64/pvtime.c > +++ b/arm/aarch64/pvtime.c > @@ -14,15 +14,15 @@ static int pvtime__alloc_region(struct kvm *kvm) > char *mem; > int ret = 0; > > - mem = mmap(NULL, ARM_PVTIME_BASE, PROT_RW, > + mem = mmap(NULL, ARM_PVTIME_SIZE, PROT_RW, > MAP_ANON_NORESERVE, -1, 0); > if (mem == MAP_FAILED) > return -errno; > > ret = kvm__register_ram(kvm, ARM_PVTIME_BASE, > - ARM_PVTIME_BASE, mem); > + ARM_PVTIME_SIZE, mem); > if (ret) { > - munmap(mem, ARM_PVTIME_BASE); > + munmap(mem, ARM_PVTIME_SIZE); > return ret; > } > > @@ -36,8 +36,8 @@ static int pvtime__teardown_region(struct kvm *kvm) > return 0; > > kvm__destroy_mem(kvm, ARM_PVTIME_BASE, > - ARM_PVTIME_BASE, usr_mem); > - munmap(usr_mem, ARM_PVTIME_BASE); > + ARM_PVTIME_SIZE, usr_mem); > + munmap(usr_mem, ARM_PVTIME_SIZE); > usr_mem = NULL; > return 0; > } > -- > 2.36.1 >
On Wed, 29 Jun 2022 11:39:05 +0100, Alexandru Elisei wrote: > pvtime uses ARM_PVTIME_BASE instead of ARM_PVTIME_SIZE for the size of the > memory region given to the guest, which causes to the following error when > creating a flash device (via the -F/--flash command line argument): > > Error: RAM (read-only) region [2000000-27fffff] would overlap RAM region [1020000-203ffff] > > The read-only region represents the guest memory where the flash image is > copied by kvmtool. The region starting at 0x102_0000 (ARM_PVTIME_BASE) is > the pvtime region, which should be 64K in size. kvmtool erroneously creates > the region to be ARM_PVTIME_BASE in size instead, and the last address > becomes: > > [...] Applied to kvmtool (master), thanks! [1/1] arm64: pvtime: Use correct region size https://git.kernel.org/will/kvmtool/c/6a1f699108e5 Cheers,
diff --git a/arm/aarch64/pvtime.c b/arm/aarch64/pvtime.c index a49cf3ed5478..2933ac7ca959 100644 --- a/arm/aarch64/pvtime.c +++ b/arm/aarch64/pvtime.c @@ -14,15 +14,15 @@ static int pvtime__alloc_region(struct kvm *kvm) char *mem; int ret = 0; - mem = mmap(NULL, ARM_PVTIME_BASE, PROT_RW, + mem = mmap(NULL, ARM_PVTIME_SIZE, PROT_RW, MAP_ANON_NORESERVE, -1, 0); if (mem == MAP_FAILED) return -errno; ret = kvm__register_ram(kvm, ARM_PVTIME_BASE, - ARM_PVTIME_BASE, mem); + ARM_PVTIME_SIZE, mem); if (ret) { - munmap(mem, ARM_PVTIME_BASE); + munmap(mem, ARM_PVTIME_SIZE); return ret; } @@ -36,8 +36,8 @@ static int pvtime__teardown_region(struct kvm *kvm) return 0; kvm__destroy_mem(kvm, ARM_PVTIME_BASE, - ARM_PVTIME_BASE, usr_mem); - munmap(usr_mem, ARM_PVTIME_BASE); + ARM_PVTIME_SIZE, usr_mem); + munmap(usr_mem, ARM_PVTIME_SIZE); usr_mem = NULL; return 0; }
pvtime uses ARM_PVTIME_BASE instead of ARM_PVTIME_SIZE for the size of the memory region given to the guest, which causes to the following error when creating a flash device (via the -F/--flash command line argument): Error: RAM (read-only) region [2000000-27fffff] would overlap RAM region [1020000-203ffff] The read-only region represents the guest memory where the flash image is copied by kvmtool. The region starting at 0x102_0000 (ARM_PVTIME_BASE) is the pvtime region, which should be 64K in size. kvmtool erroneously creates the region to be ARM_PVTIME_BASE in size instead, and the last address becomes: ARM_PVTIME_BASE + ARM_PVTIME_BASE - 1 = 0x102_0000 + 0x102_0000 - 1 = 0x203_ffff which corresponds to the end of the region from the error message. Do the right thing and make the pvtime memory region ARM_PVTIME_SIZE = 64K bytes, as it was intended. Fixes: 7d4671e5d372 ("aarch64: Add stolen time support") Reported-by: Pierre Gondois <pierre.gondois@arm.com> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arm/aarch64/pvtime.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)