On Fri, Sep 05, 2014 at 02:25:05PM +0200, David Marchand wrote:
> Hello Stefan,
>
> On 09/05/2014 12:29 PM, Stefan Hajnoczi wrote:
> >On Thu, Sep 04, 2014 at 02:51:01PM +0200, David Marchand wrote:
> >>+ "server using a different protocol please check your setup\n");
> >>+ qemu_chr_delete(s->server_chr);
> >>+ s->server_chr = NULL;
> >>+ return;
> >>+ }
> >>+
> >>+ IVSHMEM_DPRINTF("version check ok, finish init and switch to real chardev "
> >>+ "handler\n");
> >>+
> >>+ pci_register_bar(dev, 2, s->ivshmem_attr, &s->bar);
> >
> >Not sure if it is okay to delay PCI initialization to a fd hander
> >callback.
> >
> >If the version message is too slow the guest could see the PCI adapter
> >without the BAR!
> >
> >Did you move this code in order to prevent the guest from accessing the
> >device before it has connected to the server? Perhaps the device needs
> >a state field that tracks whether or not it is ready for operation. Any
> >access before RUNNING state is reached will be ignored (?).
>
> Yes, exactly.
>
> There already is a synchronisation mechanism described in the documentation:
> "When using the server, since the server is a separate process, the VM ID
> will only be set when the device is ready (shared memory is received from
> the server and accessible via the device). If the device is not ready, the
> IVPosition will return -1.
> Applications should ensure that they have a valid VM ID before accessing the
> shared memory."
>
> So actually, this move is unneeded if ivshmem users comply to this.
>
> I will let the init stuff (pci_register_bar + gmalloc) where it was before,
> ivshmem_check_version will only switch the chardev handler.
>
> What do you think about this ?
Sounds good.
Attachment:
pgpEd9VfFom_V.pgp
Description: PGP signature
