diff mbox series

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

Message ID 1621065983-18305-1-git-send-email-maobibo@loongson.cn (mailing list archive)
State New, archived
Headers show
Series hw/display/qxl: Set pci rom address aligned with page size | expand

Commit Message

maobibo May 15, 2021, 8:06 a.m. UTC
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(-)

Comments

Gerd Hoffmann May 17, 2021, 7:19 a.m. UTC | #1
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
maobibo May 18, 2021, 1:06 a.m. UTC | #2
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
>
Gerd Hoffmann May 18, 2021, 7:03 a.m. UTC | #3
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
maobibo May 18, 2021, 7:14 a.m. UTC | #4
在 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
>
Gerd Hoffmann May 18, 2021, 7:37 a.m. UTC | #5
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
maobibo May 18, 2021, 8:32 a.m. UTC | #6
在 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
>
diff mbox series

Patch

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)