[PATCH] hw/display/qxl: Set pci rom address aligned with page size

Bibo Mao posted 1 patch 2 years, 11 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1621065983-18305-1-git-send-email-maobibo@loongson.cn
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
hw/display/qxl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] hw/display/qxl: Set pci rom address aligned with page size
Posted by Bibo Mao 2 years, 11 months ago
From: maobibo <maobibo@loongson.cn>

pci memory bar size should be aligned with page size, else it will
not be effective memslot when running in kvm mode.

This patch set qxl pci rom size aligned with page size of host
machine.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/display/qxl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 2ba7563..c84f45e 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -317,11 +317,11 @@ static uint32_t qxl_crc32(const uint8_t *p, unsigned len)
 
 static ram_addr_t qxl_rom_size(void)
 {
-#define QXL_REQUIRED_SZ (sizeof(QXLRom) + sizeof(QXLModes) + sizeof(qxl_modes))
-#define QXL_ROM_SZ 8192
+    uint32_t rom_size;
 
-    QEMU_BUILD_BUG_ON(QXL_REQUIRED_SZ > QXL_ROM_SZ);
-    return QXL_ROM_SZ;
+    rom_size = sizeof(QXLRom) + sizeof(QXLModes) + sizeof(qxl_modes);
+    rom_size = QEMU_ALIGN_UP(rom_size, qemu_real_host_page_size);
+    return rom_size;
 }
 
 static void init_qxl_rom(PCIQXLDevice *d)
-- 
1.8.3.1


Re: [PATCH] hw/display/qxl: Set pci rom address aligned with page size
Posted by Gerd Hoffmann 2 years, 11 months ago
On Sat, May 15, 2021 at 04:06:23PM +0800, Bibo Mao wrote:
> From: maobibo <maobibo@loongson.cn>
> 
> pci memory bar size should be aligned with page size, else it will
> not be effective memslot when running in kvm mode.
> 
> This patch set qxl pci rom size aligned with page size of host
> machine.

What is the exact problem you are trying to fix here?

take care,
  Gerd


Re: [PATCH] hw/display/qxl: Set pci rom address aligned with page size
Posted by maobibo 2 years, 11 months ago
Sorry I do not state the background clearly.

Page size is 16K on my MIPS machine, and it supports running
guest OS in kvm mode and qxl vga card can used for VM. Qxl pci
rom size is 8K, smaller than 16K page size on host system, it
fails to be added into  memslot in kvm mode since size of
the pci memory space is not page aligned. Here is code in linux,
it requires memory_size and guest_phys_addr is page size aligned.

int __kvm_set_memory_region(struct kvm *kvm,
                            const struct kvm_userspace_memory_region *mem)
{
        struct kvm_memory_slot old, new;
        struct kvm_memory_slot *tmp;
        enum kvm_mr_change change;
        int as_id, id;
        int r;

        r = check_memory_region_flags(mem);
        if (r)
                return r;

        as_id = mem->slot >> 16;
        id = (u16)mem->slot;

        /* General sanity checks */
        if (mem->memory_size & (PAGE_SIZE - 1))
                return -EINVAL;
        if (mem->guest_phys_addr & (PAGE_SIZE - 1))
                return -EINVAL;


regards
bibo, mao

在 2021年05月17日 15:19, Gerd Hoffmann 写道:
> On Sat, May 15, 2021 at 04:06:23PM +0800, Bibo Mao wrote:
>> From: maobibo <maobibo@loongson.cn>
>>
>> pci memory bar size should be aligned with page size, else it will
>> not be effective memslot when running in kvm mode.
>>
>> This patch set qxl pci rom size aligned with page size of host
>> machine.
> 
> What is the exact problem you are trying to fix here?
> 
> take care,
>   Gerd
> 


Re: [PATCH] hw/display/qxl: Set pci rom address aligned with page size
Posted by Gerd Hoffmann 2 years, 11 months ago
On Tue, May 18, 2021 at 09:06:31AM +0800, maobibo wrote:
> Sorry I do not state the background clearly.
> 
> Page size is 16K on my MIPS machine, and it supports running
> guest OS in kvm mode and qxl vga card can used for VM.

Ok.  Please add that to the commit message.

Also there is no need to rewrite the function and drop the
BUILD_BUG_ON().  Just using "return QEMU_ALIGN_UP(QXL_ROM_SZ, pagesize)"
should work fine.

Is the host page size fixed on mips?

take care,
  Gerd


Re: [PATCH] hw/display/qxl: Set pci rom address aligned with page size
Posted by maobibo 2 years, 11 months ago

在 2021年05月18日 15:03, Gerd Hoffmann 写道:
> On Tue, May 18, 2021 at 09:06:31AM +0800, maobibo wrote:
>> Sorry I do not state the background clearly.
>>
>> Page size is 16K on my MIPS machine, and it supports running
>> guest OS in kvm mode and qxl vga card can used for VM.
> 
> Ok.  Please add that to the commit message.
> 
> Also there is no need to rewrite the function and drop the
> BUILD_BUG_ON().  Just using "return QEMU_ALIGN_UP(QXL_ROM_SZ, pagesize)"
> should work fine.
ok, I will refresh the patch.

> 
> Is the host page size fixed on mips?

No, it is not fixed on mips, and it can be selected by linux kernel config.

regards
bibo, mao
> 
> take care,
>   Gerd
> 


Re: [PATCH] hw/display/qxl: Set pci rom address aligned with page size
Posted by Gerd Hoffmann 2 years, 11 months ago
  Hi,

> > Is the host page size fixed on mips?
> 
> No, it is not fixed on mips, and it can be selected by linux kernel config.

Hmm.  So the rom size can differ depending on host kernel config.
Which is bad.  It'll break live migration between hosts with
different page sizes (or wouldn't that work anyway for other reasons?).

What page sizes are supported on mips?  4k and 16k I assume?
So maybe just use 16k unconditionally on mips?

take care,
  Gerd


Re: [PATCH] hw/display/qxl: Set pci rom address aligned with page size
Posted by maobibo 2 years, 11 months ago

在 2021年05月18日 15:37, Gerd Hoffmann 写道:
>   Hi,
> 
>>> Is the host page size fixed on mips?
>>
>> No, it is not fixed on mips, and it can be selected by linux kernel config.
> 
> Hmm.  So the rom size can differ depending on host kernel config.
> Which is bad.  It'll break live migration between hosts with
> different page sizes (or wouldn't that work anyway for other reasons?).
yes, there will be live migration problem between hosts with different page sizes, if page size of guest OS is different with host system, there will be live migration issue also. 
> 
> What page sizes are supported on mips?  4k and 16k I assume?
> So maybe just use 16k unconditionally on mips?
Different mips vendors have different kernel config, there is no general kernel config for all vendors now.

However it is fixed for one vendor. On my Loongson mips box, 16K page size is used. However other vendors have their own binary software code and kernel config.

regards
bibo, mao
> 
> take care,
>   Gerd
>