From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58544) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxdUO-0007rH-30 for qemu-devel@nongnu.org; Thu, 19 Jun 2014 10:39:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WxdUG-0001wp-JY for qemu-devel@nongnu.org; Thu, 19 Jun 2014 10:38:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1152) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxdUG-0001wj-BB for qemu-devel@nongnu.org; Thu, 19 Jun 2014 10:38:48 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5JEcl2e024505 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Thu, 19 Jun 2014 10:38:47 -0400 Date: Thu, 19 Jun 2014 17:39:15 +0300 From: "Michael S. Tsirkin" Message-ID: <20140619143915.GA10359@redhat.com> References: <1403185941-19561-1-git-send-email-marcel.a@redhat.com> <1403185941-19561-3-git-send-email-marcel.a@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1403185941-19561-3-git-send-email-marcel.a@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/3] hw/pcie: implement power controller functionality List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum Cc: qemu-devel@nongnu.org On Thu, Jun 19, 2014 at 04:52:20PM +0300, Marcel Apfelbaum wrote: > It is needed by hot-unplug in order to get an indication > from the OS when the device can be physically detached. > > Signed-off-by: Marcel Apfelbaum > --- > hw/pci/pcie.c | 15 ++++++++++++++- > include/hw/pci/pcie_regs.h | 2 ++ > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index ae92f00..f8bf515 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -292,16 +292,19 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot) > PCI_EXP_SLTCAP_HPC | > PCI_EXP_SLTCAP_PIP | > PCI_EXP_SLTCAP_AIP | > + PCI_EXP_SLTCAP_PCP | > PCI_EXP_SLTCAP_ABP); > > pci_word_test_and_clear_mask(dev->config + pos + PCI_EXP_SLTCTL, > PCI_EXP_SLTCTL_PIC | > + PCI_EXP_SLTCTL_PCC | > PCI_EXP_SLTCTL_AIC); > pci_word_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCTL, > PCI_EXP_SLTCTL_PIC_OFF | > PCI_EXP_SLTCTL_AIC_OFF); > pci_word_test_and_set_mask(dev->wmask + pos + PCI_EXP_SLTCTL, > PCI_EXP_SLTCTL_PIC | > + PCI_EXP_SLTCTL_PCC | > PCI_EXP_SLTCTL_AIC | > PCI_EXP_SLTCTL_HPIE | > PCI_EXP_SLTCTL_CCIE | Need to disable for compat types? > @@ -327,21 +330,31 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot) > void pcie_cap_slot_reset(PCIDevice *dev) > { > uint8_t *exp_cap = dev->config + dev->exp.exp_cap; > + PCIDevice *slot_dev = pci_bridge_get_sec_bus(PCI_BRIDGE(dev))->devices[0]; What does this mean? Downstream port? A switch can have multiple downstream ports at any slot #. > + int pic; uint16_t please. > + > + pic = slot_dev ? PCI_EXP_SLTCTL_PIC_ON : PCI_EXP_SLTCTL_PIC_OFF; > > PCIE_DEV_PRINTF(dev, "reset\n"); > > pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTCTL, > PCI_EXP_SLTCTL_EIC | > PCI_EXP_SLTCTL_PIC | > + PCI_EXP_SLTCTL_PCC | > PCI_EXP_SLTCTL_AIC | > PCI_EXP_SLTCTL_HPIE | > PCI_EXP_SLTCTL_CCIE | > PCI_EXP_SLTCTL_PDCE | > PCI_EXP_SLTCTL_ABPE); > pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTCTL, > - PCI_EXP_SLTCTL_PIC_OFF | > + pic | > PCI_EXP_SLTCTL_AIC_OFF); > > + if (!slot_dev) { > + pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTCTL, > + PCI_EXP_SLTCTL_PCC); I dislike it when we clear bits then set them back. Please just add else here. > + } > + Need to disable for compat types? > pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA, > PCI_EXP_SLTSTA_EIS |/* on reset, > the lock is released */ > diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h > index 4d123d9..652d9fc 100644 > --- a/include/hw/pci/pcie_regs.h > +++ b/include/hw/pci/pcie_regs.h > @@ -57,6 +57,8 @@ > #define PCI_EXP_SLTCTL_PIC_SHIFT (ffs(PCI_EXP_SLTCTL_PIC) - 1) > #define PCI_EXP_SLTCTL_PIC_OFF \ > (PCI_EXP_SLTCTL_IND_OFF << PCI_EXP_SLTCTL_PIC_SHIFT) > +#define PCI_EXP_SLTCTL_PIC_ON \ > + (PCI_EXP_SLTCTL_IND_ON << PCI_EXP_SLTCTL_PIC_SHIFT) > > #define PCI_EXP_SLTCTL_SUPPORTED \ > (PCI_EXP_SLTCTL_ABPE | \ > -- > 1.8.3.1