On Tue, Jul 18, 2023 at 05:05:12PM -0300, Daniel Henrique Barboza wrote:
>
>
> On 7/12/23 13:39, Sunil V L wrote:
> > PCIe High MMIO base is actually dynamic and fixed at
> > run time based on the RAM configured. Currently, this is
> > not part of the memmap and kept in separate static variable
> > in virt.c. However, ACPI code also needs this information
> > to populate DSDT. So, once the base is discovered, merge
> > this into the final memmap which can be used to create
> > ACPI tables later.
> >
> > Signed-off-by: Sunil V L <suni...@ventanamicro.com>
> > ---
> > hw/riscv/virt.c | 31 ++++++++++++++++++++++++++++++-
> > include/hw/riscv/virt.h | 9 +++++++--
> > 2 files changed, 37 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index f6067db8ec..7aee06f021 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -84,6 +84,22 @@ static const MemMapEntry virt_memmap[] = {
> > static MemMapEntry virt_high_pcie_memmap;
> > +/*
> > + * virt_memmap doesn't include floating High Mem IO address entry. To
> > enable
> > + * code organization in multiple files (ex: ACPI), it is better to have
> > single
> > + * memmap which has complete information.
> > + *
> > + * VIRT_HIGH_PCIE_MMIO is always greater than the last memmap entry and
> > hence
> > + * full_virt_memmap is capable of holding both virt_memmap and
> > + * VIRT_HIGH_PCIE_MMIO entry.
> > + *
> > + * The values for these floating entries will be updated when top of RAM is
> > + * discovered.
> > + */
> > +static MemMapEntry full_virt_memmap[] = {
> > + [VIRT_HIGH_PCIE_MMIO] = { 0x0, 0 },
> > +};
> > +
> > #define VIRT_FLASH_SECTOR_SIZE (256 * KiB)
> > static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s,
> > @@ -1444,7 +1460,20 @@ static void virt_machine_init(MachineState *machine)
> > ROUND_UP(virt_high_pcie_memmap.base,
> > virt_high_pcie_memmap.size);
> > }
> > - s->memmap = virt_memmap;
> > + /*
> > + * Initialize the floating values in full memory map
> > + */
> > + full_virt_memmap[VIRT_HIGH_PCIE_MMIO].base =
> > virt_high_pcie_memmap.base;
> > + full_virt_memmap[VIRT_HIGH_PCIE_MMIO].size =
> > virt_high_pcie_memmap.size;
> > +
> > + s->memmap = full_virt_memmap;
> > + /*
> > + * Copy the base virt_memmap entries to full memmap
> > + */
> > + for (i = 0; i < ARRAY_SIZE(virt_memmap); i++) {
> > + s->memmap[i] = virt_memmap[i];
> > + }
> > +
>
> This change here kind of convinces me of the point I made earlier in patch 2:
> we can simplify gpex_pcie_init() to use just the RISCVVirtState as a parameter
> and get everything else from it.
>
> It's also something for a follow-up. As for this patch:
>
> Reviewed-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
>
Thanks Daniel. I agree. I can send another follow-up patch to simplify
gpex_pcie_init.
Thanks,
Sunil
