From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH v1 01/10] xen/pt: Use xen_host_pci_get_[byte|word] instead of dev.config Date: Fri, 17 Jul 2015 16:43:12 +0100 Message-ID: References: <1435866681-18468-1-git-send-email-konrad.wilk@oracle.com> <1435866681-18468-2-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZG7oZ-00006p-1P for xen-devel@lists.xenproject.org; Fri, 17 Jul 2015 15:44:43 +0000 In-Reply-To: <1435866681-18468-2-git-send-email-konrad.wilk@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xenproject.org, qemu-devel@nongnu.org, JBeulich@suse.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On Thu, 2 Jul 2015, Konrad Rzeszutek Wilk wrote: > During init time we treat the dev.config area as a cache > of the host view. However during execution time we treat it > as guest view (by the generic PCI API). We need to sync Xen's > code to the generic PCI API view. This is the first step > by replacing all of the code that uses dev.config or > pci_get_[byte|word] to get host value to actually use the > xen_host_pci_get_[byte|word] functions. > > Interestingly in 'xen_pt_ptr_reg_init' we also needed to swap > reg_field from uint32_t to uint8_t - since the access is only > for one byte not four bytes. We can split this as a seperate > patch however we would have to use a cast to thwart compiler > warnings in the meantime. > > We also truncated 'flags' to 'flag' to make the code fit within > the 80 characters. > > Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Stefano Stabellini > hw/xen/xen_pt.c | 24 +++++++++++--- > hw/xen/xen_pt_config_init.c | 77 +++++++++++++++++++++++++++++++-------------- > 2 files changed, 73 insertions(+), 28 deletions(-) > > diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c > index ea1ceda..7575311 100644 > --- a/hw/xen/xen_pt.c > +++ b/hw/xen/xen_pt.c > @@ -689,7 +689,7 @@ static int xen_pt_initfn(PCIDevice *d) > { > XenPCIPassthroughState *s = XEN_PT_DEVICE(d); > int rc = 0; > - uint8_t machine_irq = 0; > + uint8_t machine_irq = 0, scratch; > uint16_t cmd = 0; > int pirq = XEN_PT_UNASSIGNED_PIRQ; > > @@ -735,7 +735,12 @@ static int xen_pt_initfn(PCIDevice *d) > } > > /* Bind interrupt */ > - if (!s->dev.config[PCI_INTERRUPT_PIN]) { > + rc = xen_host_pci_get_byte(&s->real_device, PCI_INTERRUPT_PIN, &scratch); > + if (rc) { > + XEN_PT_ERR(d, "Failed to read PCI_INTERRUPT_PIN! (rc:%d)\n", rc); > + scratch = 0; > + } > + if (!scratch) { > XEN_PT_LOG(d, "no pin interrupt\n"); > goto out; > } > @@ -785,8 +790,19 @@ static int xen_pt_initfn(PCIDevice *d) > > out: > if (cmd) { > - xen_host_pci_set_word(&s->real_device, PCI_COMMAND, > - pci_get_word(d->config + PCI_COMMAND) | cmd); > + uint16_t val; > + > + rc = xen_host_pci_get_word(&s->real_device, PCI_COMMAND, &val); > + if (rc) { > + XEN_PT_ERR(d, "Failed to read PCI_COMMAND! (rc: %d)\n", rc); > + } else { > + val |= cmd; > + rc = xen_host_pci_set_word(&s->real_device, PCI_COMMAND, val); > + if (rc) { > + XEN_PT_ERR(d, "Failed to write PCI_COMMAND val=0x%x!(rc: %d)\n", > + val, rc); > + } > + } > } > > memory_listener_register(&s->memory_listener, &s->dev.bus_master_as); > diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c > index 21d4938..e34f9f8 100644 > --- a/hw/xen/xen_pt_config_init.c > +++ b/hw/xen/xen_pt_config_init.c > @@ -800,15 +800,21 @@ static XenPTRegInfo xen_pt_emu_reg_vendor[] = { > static inline uint8_t get_capability_version(XenPCIPassthroughState *s, > uint32_t offset) > { > - uint8_t flags = pci_get_byte(s->dev.config + offset + PCI_EXP_FLAGS); > - return flags & PCI_EXP_FLAGS_VERS; > + uint8_t flag; > + if (xen_host_pci_get_byte(&s->real_device, offset + PCI_EXP_FLAGS, &flag)) { > + return 0; > + } > + return flag & PCI_EXP_FLAGS_VERS; > } > > static inline uint8_t get_device_type(XenPCIPassthroughState *s, > uint32_t offset) > { > - uint8_t flags = pci_get_byte(s->dev.config + offset + PCI_EXP_FLAGS); > - return (flags & PCI_EXP_FLAGS_TYPE) >> 4; > + uint8_t flag; > + if (xen_host_pci_get_byte(&s->real_device, offset + PCI_EXP_FLAGS, &flag)) { > + return 0; > + } > + return (flag & PCI_EXP_FLAGS_TYPE) >> 4; > } > > /* initialize Link Control register */ > @@ -857,8 +863,14 @@ static int xen_pt_linkctrl2_reg_init(XenPCIPassthroughState *s, > reg_field = XEN_PT_INVALID_REG; > } else { > /* set Supported Link Speed */ > - uint8_t lnkcap = pci_get_byte(s->dev.config + real_offset - reg->offset > - + PCI_EXP_LNKCAP); > + uint8_t lnkcap; > + int rc; > + rc = xen_host_pci_get_byte(&s->real_device, > + real_offset - reg->offset + PCI_EXP_LNKCAP, > + &lnkcap); > + if (rc) { > + return rc; > + } > reg_field |= PCI_EXP_LNKCAP_SLS & lnkcap; > } > > @@ -1039,13 +1051,15 @@ static int xen_pt_msgctrl_reg_init(XenPCIPassthroughState *s, > XenPTRegInfo *reg, uint32_t real_offset, > uint32_t *data) > { > - PCIDevice *d = &s->dev; > XenPTMSI *msi = s->msi; > - uint16_t reg_field = 0; > + uint16_t reg_field; > + int rc; > > /* use I/O device register's value as initial value */ > - reg_field = pci_get_word(d->config + real_offset); > - > + rc = xen_host_pci_get_word(&s->real_device, real_offset, ®_field); > + if (rc) { > + return rc; > + } > if (reg_field & PCI_MSI_FLAGS_ENABLE) { > XEN_PT_LOG(&s->dev, "MSI already enabled, disabling it first\n"); > xen_host_pci_set_word(&s->real_device, real_offset, > @@ -1411,12 +1425,14 @@ static int xen_pt_msixctrl_reg_init(XenPCIPassthroughState *s, > XenPTRegInfo *reg, uint32_t real_offset, > uint32_t *data) > { > - PCIDevice *d = &s->dev; > - uint16_t reg_field = 0; > + uint16_t reg_field; > + int rc; > > /* use I/O device register's value as initial value */ > - reg_field = pci_get_word(d->config + real_offset); > - > + rc = xen_host_pci_get_word(&s->real_device, real_offset, ®_field); > + if (rc) { > + return rc; > + } > if (reg_field & PCI_MSIX_FLAGS_ENABLE) { > XEN_PT_LOG(&s->dev, "MSIX already enabled, disabling it first\n"); > xen_host_pci_set_word(&s->real_device, real_offset, > @@ -1511,8 +1527,7 @@ static int xen_pt_vendor_size_init(XenPCIPassthroughState *s, > const XenPTRegGroupInfo *grp_reg, > uint32_t base_offset, uint8_t *size) > { > - *size = pci_get_byte(s->dev.config + base_offset + 0x02); > - return 0; > + return xen_host_pci_get_byte(&s->real_device, base_offset + 0x02, size); > } > /* get PCI Express Capability Structure register group size */ > static int xen_pt_pcie_size_init(XenPCIPassthroughState *s, > @@ -1591,12 +1606,15 @@ static int xen_pt_msi_size_init(XenPCIPassthroughState *s, > const XenPTRegGroupInfo *grp_reg, > uint32_t base_offset, uint8_t *size) > { > - PCIDevice *d = &s->dev; > uint16_t msg_ctrl = 0; > uint8_t msi_size = 0xa; > + int rc; > > - msg_ctrl = pci_get_word(d->config + (base_offset + PCI_MSI_FLAGS)); > - > + rc = xen_host_pci_get_word(&s->real_device, base_offset + PCI_MSI_FLAGS, > + &msg_ctrl); > + if (rc) { > + return rc; > + } > /* check if 64-bit address is capable of per-vector masking */ > if (msg_ctrl & PCI_MSI_FLAGS_64BIT) { > msi_size += 4; > @@ -1739,11 +1757,14 @@ static int xen_pt_ptr_reg_init(XenPCIPassthroughState *s, > XenPTRegInfo *reg, uint32_t real_offset, > uint32_t *data) > { > - int i; > - uint8_t *config = s->dev.config; > - uint32_t reg_field = pci_get_byte(config + real_offset); > + int i, rc; > + uint8_t reg_field; > uint8_t cap_id = 0; > > + rc = xen_host_pci_get_byte(&s->real_device, real_offset, ®_field); > + if (rc) { > + return rc; > + } > /* find capability offset */ > while (reg_field) { > for (i = 0; xen_pt_emu_reg_grps[i].grp_size != 0; i++) { > @@ -1752,7 +1773,11 @@ static int xen_pt_ptr_reg_init(XenPCIPassthroughState *s, > continue; > } > > - cap_id = pci_get_byte(config + reg_field + PCI_CAP_LIST_ID); > + rc = xen_host_pci_get_byte(&s->real_device, > + reg_field + PCI_CAP_LIST_ID, &cap_id); > + if (rc) { > + return rc; > + } > if (xen_pt_emu_reg_grps[i].grp_id == cap_id) { > if (xen_pt_emu_reg_grps[i].grp_type == XEN_PT_GRP_TYPE_EMU) { > goto out; > @@ -1763,7 +1788,11 @@ static int xen_pt_ptr_reg_init(XenPCIPassthroughState *s, > } > > /* next capability */ > - reg_field = pci_get_byte(config + reg_field + PCI_CAP_LIST_NEXT); > + rc = xen_host_pci_get_byte(&s->real_device, > + reg_field + PCI_CAP_LIST_NEXT, ®_field); > + if (rc) { > + return rc; > + } > } > > out: > -- > 2.1.0 >