[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support |
Date: |
Thu, 17 Apr 2014 18:01:35 +0300 |
On Thu, Apr 17, 2014 at 03:41:56PM +0200, Hannes Reinecke wrote:
> On 04/17/2014 02:07 PM, Andreas Färber wrote:
> > Am 17.04.2014 08:20, schrieb Hannes Reinecke:
> >> On 04/16/2014 07:52 PM, Michael S. Tsirkin wrote:
> >>> On Wed, Apr 16, 2014 at 07:47:37PM +0200, Andreas Färber wrote:
> >>>> Am 16.04.2014 19:40, schrieb Michael S. Tsirkin:
> >>>>> On Wed, Apr 16, 2014 at 06:48:08PM +0200, Andreas Färber wrote:
> >>>>>> Am 16.04.2014 18:32, schrieb Alexander Graf:
> >>>>>>>
> >>>>>>> On 16.04.14 16:44, Hannes Reinecke wrote:
> >>>>>>>> MSI-X support has been fixed in qemu, so we can enable it again.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Hannes Reinecke <address@hidden>
> >>>>>>>> ---
> >>>>>>>> hw/scsi/megasas.c | 19 ++++++-------------
> >>>>>>>> 1 file changed, 6 insertions(+), 13 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> >>>>>>>> index 1781525..df45286 100644
> >>>>>>>> --- a/hw/scsi/megasas.c
> >>>>>>>> +++ b/hw/scsi/megasas.c
> >>>>>>>> @@ -2084,7 +2084,8 @@ static const VMStateDescription
> >>>>>>>> vmstate_megasas = {
> >>>>>>>> .minimum_version_id = 0,
> >>>>>>>> .minimum_version_id_old = 0,
> >>>>>>>> .fields = (VMStateField[]) {
> >>>>>>>> - VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
> >>>>>>>> + VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
> >>>>>>>> + VMSTATE_MSIX(parent_obj, MegasasState),
> >>>>>>>
> >>>>>>> This requires a version change for vmstate, no?
> >>>>>>
> >>>>>> The PCI -> PCIE change yes. mst might have objections for bumping an
> >>>>>> x86
> >>>>>> PCI device?
> >>>>>
> >>>>> Yes.
> >>>>> It should be possible to make this conditional on having a pcie
> >>>>> capability, and disable pcie capability for old pc versions.
> >>>>
> >>>> I did have a patch merging their vmstate_ structs based on some test in
> >>>> the series I ping'ed, unfortunately the difference was mostly config
> >>>> space size, so not immediately obvious how to set via compat_props, but
> >>>> likely solvable somehow.
> >>>
> >>> Check pci_is_express ?
> >>>
> >>>>>> The MSIX addition should be safe AFAICT since old versions would not
> >>>>>> enable MSI-X.
> >>>>>
> >>>>> Yes but I don't see a code like this - where's code in PC
> >>>>> disabling msix for old versions?
> >>>>
> >>>> The default value for use_msix (use-msix?) in v2 is already false, so no
> >>>> need to set it to false again in compat_props?
> >>>>
> >>>> Andreas
> >>>
> >>> Ah, I didn't notice that. Sure, if it's off by default there's
> >>> no need for compat code.
> >>>
> >> And the net result is ... what?
> >> Do I need to change the code?
> >
> > Yes, NAK for this patch as is.
> >
> > Why are you changing VMSTATE_PCI_DEVICE() to VMSTATE_PCIE_DEVICE()? Was
> > it always a PCIe device and you're fixing that now? Then put it into its
> > own patch with proper explanation, setting pdc->is_express = 1 *and*
> > assure there via some compatibility property (?) that migration from
> > 2.0.0-rc3 to the patched version succeeds (which would probably still
> > break backwards migration that mst was interested in though).
> >
> The thing was always PCIe; it's just that at the time I've
> originally implemented it MSI-X support was broken, and PCIe even
> more so. So I figured to take the easy way out and implement it as a
> normal PCI device.
>
> I'm happy to continue with having the original implementation at
> PCI, and only moving to PCIe with the gen2 emulation.
>
> Would that be okay?
>
> Cheers,
>
> Hannes
And then gen1 and gen2 get different names? That's okay.
Alternatively, I think it's reasonable to add a property
which specifies PCI or PCIE.
You then force compatible behaviour for old machine types,
see e.g. PC_COMPAT_XX macros in include/hw/i386/pc.h
> --
> Dr. Hannes Reinecke zSeries & Storage
> address@hidden +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
- [Qemu-devel] [PATCH 1/7] megasas: Implement LD_LIST_QUERY, (continued)
- [Qemu-devel] [PATCH 1/7] megasas: Implement LD_LIST_QUERY, Hannes Reinecke, 2014/04/16
- [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Hannes Reinecke, 2014/04/16
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Alexander Graf, 2014/04/16
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Andreas Färber, 2014/04/16
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Michael S. Tsirkin, 2014/04/16
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Andreas Färber, 2014/04/16
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Michael S. Tsirkin, 2014/04/16
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Hannes Reinecke, 2014/04/17
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Andreas Färber, 2014/04/17
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Hannes Reinecke, 2014/04/17
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Paolo Bonzini, 2014/04/28
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Hannes Reinecke, 2014/04/28
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Paolo Bonzini, 2014/04/28
- Re: [Qemu-devel] [PATCH 2/7] megasas: Enable MSI-X support, Hannes Reinecke, 2014/04/28
[Qemu-devel] [PATCH 3/7] megasas: Add MSI support, Hannes Reinecke, 2014/04/16
[Qemu-devel] [PATCH 4/7] megasas: add MegaRAID SAS 2108 emulation, Hannes Reinecke, 2014/04/16
[Qemu-devel] [PATCH 6/7] megasas: Decode register names, Hannes Reinecke, 2014/04/16
[Qemu-devel] [PATCH 5/7] megasas: simplify trace event messages, Hannes Reinecke, 2014/04/16
[Qemu-devel] [PATCH 7/7] MAINTAINERS: mark megasas as maintained, Hannes Reinecke, 2014/04/16