Re: Re: [PATCH] kvm/arm64: Fix memory section did not set to kvm

[liucong2]

Hi Peter,

I found this issue on qmeu 4.2 with host linux 4.19, I want to

use qxl on arm64. on arm64, default page size is 64k, and the

qxl_rom_size is fixed 8192.

but when I read qxl_rom region in guest, guest os stopped and

I can see error message "load/store instruction decodeing not 

implemented" in host side. it is because qxl rom bar memory

region didn't commit to kvm.

I only try qemu 6.0 rather than the latest version because 

I meet some compile issue. commit ce7015d9e8669e

start v6.1.0-rc0, it will change the default qxl rom bar size

to 64k on my platform. then my problem disappear. but when 

others create a memory region with the size less than one

page. when it run into kvm_align_section, it return 0

again.

Regards

Cong.

       

主　题：Re: [PATCH] kvm/arm64: Fix memory section did not set to kvm            
日　期：2022-03-25 21:12            
发件人：Peter Maydell            
收件人：Cong Liu                    

       

On Fri, 18 Mar 2022 at 14:24, Cong Liuwrote:
>
> on the arm64 platform, the PAGESIZE is 64k, the default qxl rom
> bar size is 8k(QXL_ROM_SZ), in the case memory size less than
> one page size, kvm_align_section return zero,  the memory section
> did not commit kvm.

Can you give more details on how this happens? The only place
we use QXL_ROM_SZ is in the qxl_rom_size() function, and that
rounds up the value it returns to the qemu_real_host_page_size.
That change was added in commit ce7015d9e8669e, exagctly to
fix what sounds like the same problem you're hitting where
KVM is in use and the host page size is larger than 8K.
Are you using an old version of QEMU that doesn't have that fix ?

> Signed-off-by: Cong Liu
> ---
>  accel/kvm/kvm-all.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 27864dfaea..f57cab811b 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -318,6 +318,7 @@ static hwaddr kvm_align_section(MemoryRegionSection *section,
>                                  hwaddr *start)
>  {
>      hwaddr size = int128_get64(section->size);
> +    size = ROUND_UP(size, qemu_real_host_page_size);
>      hwaddr delta, aligned;
>
>      /* kvm works in page size chunks, but the function may be called

The comment we can just see starting here says:

   /* kvm works in page size chunks, but the function may be called
      with sub-page size and unaligned start address. Pad the start
      address to next and truncate size to previous page boundary. */

but your change means that's no longer true.

More generally, rounding up the size here seems dubious -- there
is no guarantee that whatever follows the small lump of RAM
in the address space is sensible to treat as really being
part of the same thing.

thanks
-- PMM