|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 1 1/8] xen/pt: Use xen_host_pci_get_[byte|word] instead of dev.config
On Mon, 29 Jun 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 <konrad.wilk@xxxxxxxxxx>
> ---
> hw/xen/xen_pt.c | 22 +++++++++++--
> hw/xen/xen_pt_config_init.c | 77
> +++++++++++++++++++++++++++++++--------------
> 2 files changed, 72 insertions(+), 27 deletions(-)
>
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index 1d256b9..2535352 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -738,7 +738,12 @@ static int xen_pt_initfn(PCIDevice *d)
> }
>
> /* Bind interrupt */
> - if (!s->dev.config[PCI_INTERRUPT_PIN]) {
> + if (xen_host_pci_get_byte(&s->real_device, PCI_INTERRUPT_PIN,
> + &machine_irq /* temp scratch */)) {
> + XEN_PT_ERR(d, "Failed to read PCI_INTERRUPT_PIN! (rc:%d)\n", rc);
printing rc, but rc is not set
> + machine_irq = 0;
> + }
I understand that machine_irq is just used as a scratch value here, but
I would rather introduce a new variable
> + if (!machine_irq) {
> XEN_PT_LOG(d, "no pin interrupt\n");
> goto out;
> }
> @@ -788,8 +793,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 {
} else { is allowed
> + val |= cmd;
> + if (xen_host_pci_set_word(&s->real_device, PCI_COMMAND, val)) {
> + XEN_PT_ERR(d, "Failed to write PCI_COMMAND val=0x%x!(rc:
> %d)\n",
> + val, rc);
rc not set but printed
> + }
> + }
> }
>
> 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
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |