[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/2] hw/pci: fixed hotplug crash when using r
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH v4 2/2] hw/pci: fixed hotplug crash when using rombar=0 with devices having romfile |
Date: |
Mon, 03 Nov 2014 14:52:20 +0200 |
On Mon, 2014-11-03 at 13:03 +0100, Markus Armbruster wrote:
> Marcel Apfelbaum <address@hidden> writes:
>
> > Hot-plugging a device that has a romfile (either supplied by user
> > or built-in) using rombar=0 option is a user error,
> > do not allow the device to be hot-plugged.
> >
> > Reviewed-by: Eric Blake <address@hidden>
> > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > ---
> > hw/pci/pci.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 36226eb..371699c 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -1942,6 +1942,15 @@ static int pci_add_option_rom(PCIDevice *pdev, bool
> > is_default_rom)
> > * for 0.11 compatibility.
> > */
> > int class = pci_get_word(pdev->config + PCI_CLASS_DEVICE);
> > +
> > + /*
> > + * Hot-plugged devices can't use the option ROM
> > + * if the rom bar is disabled.
> > + */
> > + if (DEVICE(pdev)->hotplugged) {
> > + return -1;
> > + }
> > +
> > if (class == 0x0300) {
> > rom_add_vga(pdev->romfile);
> > } else {
>
> Unlike the function's other unsuccessful returns, this one is silent.
> Intentional?
Yes, the first version included an error message, but was not accepted
as the reviewers preferred "silent drop" instead.
The main reason was that a proper error propagation mechanism
should be used.
At the time of the patch there was not such an option, but now there is.
I can add it on top of your series, preferably after is merged.
Thanks,
Marcel