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

[Peter Maydell]

On Mon, 28 Mar 2022 at 10:42, Cong Liu <liucong2@kylinos.cn> wrote:
> On 2022/3/25 23:00, Peter Maydell wrote:
> > This is correct behaviour. If the memory region is less than
> > a complete host page then it is not possible for KVM to
> > map it into the guest as directly accessible memory,
> > because that can only be done in host-page sized chunks,
> > and if the MR is a RAM region smaller than the page then
> > there simply is not enough backing RAM there to map without
> > incorrectly exposing to the guest whatever comes after the
> > contents of the MR.
>
> actually, even with fixed 8192 qxl rom bar size, the RAMBlock
> size corresponding to MemoryRegion will also be 64k.

Where does this rounding up happen? In any case, it would
still be wrong -- if the ROM bar is 8192 large then the
guest should get a fault writing to bytes past 8191, not
reads-as-written.

> so it can
> map into the guest as directly accessible memory. now it failed
> just because we use the wrong size. ROUND_UP(n, d) requires
> that d be a power of 2, it is faster than QEMU_ALIGN_UP().
> and the qemu_real_host_page_size should always a power of 2.
> seems we can use this patch and no need to fall back to "treat
> like MMIO device access".
>
> >
> > For memory regions smaller than a page, KVM and QEMU will
> > fall back to "treat like MMIO device access". As long as the

> I don't understand how it works, can you help explain or tell me
> which part of the code I should read to understand?

The KVM code in the kernel takes a fault because there is
nothing mapped at that address in the stage 2 page tables.
This results in kvm_handle_guest_abort() being called.
This function sorts out various cases it can handle
(eg "this is backed by host RAM which we need to page in")
and cases which are always errors (eg "the guest tried to
fetch an instruction from non-RAM"). For the cases of
"treat like MMIO device access" it calls io_mem_abort().
In io_mem_abort() we check whether the guest instruction that
did the load/store was a sensible one (this is the
kvm_vcpu_dabt_isvalid() check). Assuming that it was, then
we fill in some kvm_run struct fields with the parameters like
access size, address, etc (which the host CPU tells us in the
ESR_ELx syndrome register) cause an exit to userspace with
KVM_EXIT_MMIO as the reason.

In QEMU, the code in kvm_cpu_exec() has a case for the
KVM_EXIT_MMIO code. It just calls address_space_rw()
using the address, length, etc parameters that the kernel
gave us. If this is a load then the loaded data is filled
in in the kvm_run struct. Then it loops back around to do a
KVM_RUN ioctl, handing control back to the kernel.

In the kernel, in the arm64 kvm_arch_vcpu_ioctl_run()
we check whether we've just come back from a KVM_EXIT_MMIO
exit, and if so call kvm_handle_mmio_return(). If the
faulting instruction was a load, we read the data from
the kvm_run struct, sign extend as appropriate, and write
to the appropriate guest register. Then we increment the
guest program counter. Finally we start execution in the
guest in the normal way.

> the test code appended.
> it works with some differences between arm64 and x86. in x86, it
> printf rom_test->magic and rom_test->id correctly, but in arm64.
> it printf rom_test->magic correctly. when I try to print the
> rom_test->id. I get "load/store instruction decoding not
> implemented" error message.

You don't show the guest code, which is the thing that matters
here. In any case for the QXL ROM we already have the fix,
which is to make the ROM as big as the host page size.

-- PMM