[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH] hw/arm/virt: Support larger highmem MMIO regions
From: |
Shameerali Kolothum Thodi |
Subject: |
RE: [PATCH] hw/arm/virt: Support larger highmem MMIO regions |
Date: |
Wed, 29 Jan 2025 08:10:31 +0000 |
> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Wednesday, January 29, 2025 7:56 AM
> To: Matt Ochs <mochs@nvidia.com>; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>
> Cc: qemu-devel@nongnu.org; Nathan Chen <nathanc@nvidia.com>;
> ddutile@redhat.com; Nicolin Chen <nicolinc@nvidia.com>; Ankit Agrawal
> <ankita@nvidia.com>
> Subject: Re: [PATCH] hw/arm/virt: Support larger highmem MMIO regions
> >>>> + if (size < extended_memmap[VIRT_HIGH_PCIE_MMIO].size) {
> >>> Not sure it is better to fallback to default size here instead of setting
> error.
> >> I think if the user sets a value it shall be obeyed
> > Agreed.
> >
> >> Note that per the dynamic memory map algo, changing the size will also
> >> change the base address. See
> >>
> >> virt_set_high_memmap(). By the wayn why do we forbid a smaller size?
> > That’s a good point, I will remove this check.
Is there really a use case where a user will want a smaller size than default?
> >
> >>>> + object_class_property_add(oc, "highmem-mmio-size", "size",
> >>>> + virt_get_highmem_mmio_size,
> >>>> + virt_set_highmem_mmio_size,
> >>>> + NULL, NULL);
> >>>> + object_class_property_set_description(oc, "highmem-mmio-size",
> >>>> + "Set extended MMIO memory map
> >>>> size");
> >>>> +
> >>> I think this probably needs backward compatibility to keep migration
> happy.
> >>> Isn't it? See the no_highmem_compact handling.
> >> I guess if we keep the same value as default we are good. The difference
> >> with highmem_compact is it was set by default from 7.2 onwards hence
> >> changing the mmio layout. Here by default you keep the same IIUC.
> > I’m not sure I see an issue since the code is directly modifying the size
> value
> > in the extended_memmap array.
>
> I meant that if by default we keep the size value equal to 512G (the
> existing default value), I don't think we need to care about compats.
Yeah. If it has default size , it should be Ok I guess. But what if,
Your source is something like,
./qemu-new -machine virt-9.1,...,highmem-mmio-size=XXX
and has a target VM started with
./qemu-9.1 -machine virt,...
The migration will be still successful but will have memory map differences,
right?
Or the above is not considered as a valid use case and the onus of making
sure we are using it correctly with default size is on the user.
Thanks,
SHameer