Jean Guyader
2012-May-10 19:04 UTC
[PATCH] qemu-xen-traditional: Enable MSI after host sleep (v2)
After a host sleep MSI will be off on the host but qemu still thinks it''s on because of some state that have been set previously. If qemu thinks that the device has been configure already and the host MSI are disabled tell Xen to reconfigure the MSI. Signed-off-by: Jean Guyader <jean.guyader@gmail.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Stefano Stabellini
2012-May-11 16:06 UTC
Re: [PATCH] qemu-xen-traditional: Enable MSI after host sleep (v2)
On Thu, 10 May 2012, Jean Guyader wrote:> After a host sleep MSI will be off on the host but qemu still thinks > it''s on because of some state that have been set previously. > > If qemu thinks that the device has been configure already > and the host MSI are disabled tell Xen to reconfigure the MSI. > > Signed-off-by: Jean Guyader <jean.guyader@gmail.com> > > diff --git a/hw/pass-through.c b/hw/pass-through.c > index f832c5a..a6a9b7a 100644 > --- a/hw/pass-through.c > +++ b/hw/pass-through.c > @@ -3772,6 +3772,21 @@ static int pt_pmcsr_reg_write(struct pt_dev *ptdev, > return 0; > } > > +static int msi_is_enable(struct pt_dev *dev) > +{ > + uint16_t val = 0; > + uint32_t address = 0; > + if (!dev->msi) > + return 0; > + > + address = dev->msi->ctrl_offset; > + if (!address) > + return 0; > + > + val = pci_read_word(dev->pci_dev, address); > + return val & PCI_MSI_FLAGS_ENABLE; > +} > + > /* write Message Control register */ > static int pt_msgctrl_reg_write(struct pt_dev *ptdev, > struct pt_reg_tbl *cfg_entry, > @@ -3803,8 +3818,7 @@ static int pt_msgctrl_reg_write(struct pt_dev *ptdev, > /* update MSI */ > if (val & PCI_MSI_FLAGS_ENABLE) > { > - /* setup MSI pirq for the first time */ > - if (ptdev->msi->flags & MSI_FLAG_UNINIT) > + if (!msi_is_enable(ptdev)) > { > if (ptdev->msi_trans_en) { > PT_LOG("guest enabling MSI, disable MSI-INTx translation\n"); > diff --git a/hw/pt-msi.c b/hw/pt-msi.c > index 70c4023..99f9afd 100644 > --- a/hw/pt-msi.c > +++ b/hw/pt-msi.c > @@ -67,12 +67,6 @@ int pt_msi_setup(struct pt_dev *dev) > int pirq = -1; > uint8_t gvec = 0; > > - if ( !(dev->msi->flags & MSI_FLAG_UNINIT) ) > - { > - PT_LOG("Error: setup physical after initialized?? \n"); > - return -1; > - } > - > gvec = dev->msi->data & 0xFF; > if (!gvec) { > /* if gvec is 0, the guest is asking for a particular pirq thatThe patch makes sense. Only one last comment, sorry for not having noticed this before, but there might be another (flags & MSI_FLAG_UNINIT) check that should probably be changed into msi_is_enable in pt_msi_disable.
Jean Guyader
2012-May-24 09:44 UTC
Re: [PATCH] qemu-xen-traditional: Enable MSI after host sleep (v2)
On 11 May 2012 17:06, Stefano Stabellini <stefano.stabellini@eu.citrix.com>wrote:> On Thu, 10 May 2012, Jean Guyader wrote: > > After a host sleep MSI will be off on the host but qemu still thinks > > it''s on because of some state that have been set previously. > > > > If qemu thinks that the device has been configure already > > and the host MSI are disabled tell Xen to reconfigure the MSI. > > > > Signed-off-by: Jean Guyader <jean.guyader@gmail.com> > > > > diff --git a/hw/pass-through.c b/hw/pass-through.c > > index f832c5a..a6a9b7a 100644 > > --- a/hw/pass-through.c > > +++ b/hw/pass-through.c > > @@ -3772,6 +3772,21 @@ static int pt_pmcsr_reg_write(struct pt_dev > *ptdev, > > return 0; > > } > > > > +static int msi_is_enable(struct pt_dev *dev) > > +{ > > + uint16_t val = 0; > > + uint32_t address = 0; > > + if (!dev->msi) > > + return 0; > > + > > + address = dev->msi->ctrl_offset; > > + if (!address) > > + return 0; > > + > > + val = pci_read_word(dev->pci_dev, address); > > + return val & PCI_MSI_FLAGS_ENABLE; > > +} > > + > > /* write Message Control register */ > > static int pt_msgctrl_reg_write(struct pt_dev *ptdev, > > struct pt_reg_tbl *cfg_entry, > > @@ -3803,8 +3818,7 @@ static int pt_msgctrl_reg_write(struct pt_dev > *ptdev, > > /* update MSI */ > > if (val & PCI_MSI_FLAGS_ENABLE) > > { > > - /* setup MSI pirq for the first time */ > > - if (ptdev->msi->flags & MSI_FLAG_UNINIT) > > + if (!msi_is_enable(ptdev)) > > { > > if (ptdev->msi_trans_en) { > > PT_LOG("guest enabling MSI, disable MSI-INTx > translation\n"); > > diff --git a/hw/pt-msi.c b/hw/pt-msi.c > > index 70c4023..99f9afd 100644 > > --- a/hw/pt-msi.c > > +++ b/hw/pt-msi.c > > @@ -67,12 +67,6 @@ int pt_msi_setup(struct pt_dev *dev) > > int pirq = -1; > > uint8_t gvec = 0; > > > > - if ( !(dev->msi->flags & MSI_FLAG_UNINIT) ) > > - { > > - PT_LOG("Error: setup physical after initialized?? \n"); > > - return -1; > > - } > > - > > gvec = dev->msi->data & 0xFF; > > if (!gvec) { > > /* if gvec is 0, the guest is asking for a particular pirq that > > The patch makes sense. > Only one last comment, sorry for not having noticed this before, but > there might be another (flags & MSI_FLAG_UNINIT) check that should > probably be changed into msi_is_enable in pt_msi_disable. >Also I think that we should always call pt_msi_setup (the first time) even if the MSI are set in the config space. It should maybe look like my first patch. Jean _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel