[Qemu-devel] Re: [RFC PATCH 3/6] RAMBlock: Add a name field

[Alex Williamson]

On Tue, 2010-06-08 at 14:41 -0700, Chris Wright wrote:
> * Alex Williamson (address@hidden) wrote:
> > +    // XXX check duplicates
> 
> Yes, definitely. 

Yep, I was just thinking that without freeing, the uniqueness really
falls apart.  If we hotplug a nic multiple times on the source, we're
going to have multiple pci:d.f for the slot.  If we're lucky, they're
all for the same type of device and therefore the same ROM size and the
ordering will cause them to be merged into one on the target.  That
requires a lot of luck though.  Maybe once we have freeing we just blow
up and call it a device bug if it didn't free and tries to add more ram
with the same name.

> You created a notion of a hierarchical namespace,
> can this be formalized any more?  Currently scattered...

Yeah, it's pretty haphazard, I probably haven't spent enough time
working out what makes sense across all devices.  I started with
"ram.<device/driver>.<context>", where context was whatever I could
guess from the surrounding code.  Then I jumped over to "pci:d.f" (which
should at least be "pci.s:b:d.f") just because I thought the PCI address
space already provided a nicely unique layout.

> > +                char name[14];
> > +                snprintf(name, sizeof(name), "pci:%02x.%x.bar%d",
> > +                         PCI_SLOT(pci_dev->dev.devfn),
> > +                         PCI_FUNC(pci_dev->dev.devfn), i);
> > +                pci_dev->v_addrs[i].memory_index = qemu_ram_map(name,
> > +                                                  cur_region->size, 
> > virtbase);
> > +    ram_addr = qemu_ram_alloc("ram.pc.lowmem", below_4g_mem_size);
> > +        ram_addr = qemu_ram_alloc("ram.pc.highmem", above_4g_mem_size);
> > +    bios_offset = qemu_ram_alloc("ram.pc.bios", bios_size);
> > +    option_rom_offset = qemu_ram_alloc("ram.pc.rom", PC_ROM_SIZE);
> > +    char name[13];
> (13, 14...good to be consistent, maybe w/ helper like, 
> pci_ram_alloc_{bar,rom})

That probably makes sense.

> > +    snprintf(name, sizeof(name), "pci:%02x.%x.rom",
> > +             PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> > +    pdev->rom_offset = qemu_ram_alloc(name, size);
> > +    s->vram_offset = qemu_ram_alloc("ram.vga.vram", vga_ram_size);
> > +    s->fifo_offset = qemu_ram_alloc("ram.vmsvga.fifo", s->fifo_size);
> 
> So far we have:
>  ram.
>      pc.
>         lowmem
>         highmem
>         bios
>         rom
>      vga.
>          vram
>      vmsvga.
>             fifo

So maybe we say "ram." = "platform devices", things that have single
instances or are easy to split up into fixed, unique names.

>   pci.
>       D.F. (B:D.F?)
>           bar
>           rom

This one is pretty obvious.  If this is a reasonable starting point, I
can start hashing through the other archs and take a guess at what makes
sense.  Thanks,

Alex