xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Re: [Xen-devel] [PATCH v3] xen-pciback: optionally allow interrupt enable flag writes
       [not found] <202001112351.gy4c3aUU%lkp@intel.com>
@ 2020-01-13 17:11 ` Nick Desaulniers
  0 siblings, 0 replies; 4+ messages in thread
From: Nick Desaulniers @ 2020-01-13 17:11 UTC (permalink / raw)
  To: marmarek
  Cc: Juergen Gross, sstabellini, kbuild-all, kbuild test robot,
	kbuild, yuehaibing, LKML, simon, clang-built-linux, jbeulich,
	xen-devel, Boris Ostrovsky

Hi Marek,
Below is a report from 0day bot build w/ Clang. The warning looks
legit, can you please take a look? Apologies if this has already been
reported.

On Sat, Jan 11, 2020 at 7:48 AM kbuild test robot <lkp@intel.com> wrote:
>
> CC: kbuild-all@lists.01.org
> In-Reply-To: <20200111034347.5270-1-marmarek@invisiblethingslab.com>
> References: <20200111034347.5270-1-marmarek@invisiblethingslab.com>
> TO: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
> CC: xen-devel@lists.xenproject.org, "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>, Jan Beulich <jbeulich@suse.com>, Simon Gaiser <simon@invisiblethingslab.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Juergen Gross <jgross@suse.com>, Stefano Stabellini <sstabellini@kernel.org>, YueHaibing <yuehaibing@huawei.com>, open list <linux-kernel@vger.kernel.org>, "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>, Jan Beulich <jbeulich@suse.com>, Simon Gaiser <simon@invisiblethingslab.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Juergen Gross <jgross@suse.com>, Stefano Stabellini <sstabellini@kernel.org>, YueHaibing <yuehaibing@huawei.com>, open list <linux-kernel@vger.kernel.org>
> CC: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>, Jan Beulich <jbeulich@suse.com>, Simon Gaiser <simon@invisiblethingslab.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Juergen Gross <jgross@suse.com>, Stefano Stabellini <sstabellini@kernel.org>, YueHaibing <yuehaibing@huawei.com>, open list <linux-kernel@vger.kernel.org>
>
> Hi "Marek,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on xen-tip/linux-next]
> [also build test WARNING on linux/master linus/master v5.5-rc5 next-20200110]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/Marek-Marczykowski-G-recki/xen-pciback-optionally-allow-interrupt-enable-flag-writes/20200111-162243
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
> config: x86_64-allyesconfig (attached as .config)
> compiler: clang version 10.0.0 (git://gitmirror/llvm_project 016bf03ef6fcd9dce43b0c17971f76323f07a684)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/xen/xen-pciback/conf_space_header.c:121:19: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
>                    if ((cmd->val ^ val) & PCI_COMMAND_INTX_DISABLE) {
>                                    ^~~
>    drivers/xen/xen-pciback/conf_space_header.c:65:9: note: initialize the variable 'val' to silence this warning
>            u16 val;
>                   ^
>                    = 0
>    1 warning generated.
>
> vim +/val +121 drivers/xen/xen-pciback/conf_space_header.c
>
>     60
>     61  static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
>     62  {
>     63          struct xen_pcibk_dev_data *dev_data;
>     64          int err;
>     65          u16 val;
>     66          struct pci_cmd_info *cmd = data;
>     67
>     68          dev_data = pci_get_drvdata(dev);
>     69          if (!pci_is_enabled(dev) && is_enable_cmd(value)) {
>     70                  if (unlikely(verbose_request))
>     71                          printk(KERN_DEBUG DRV_NAME ": %s: enable\n",
>     72                                 pci_name(dev));
>     73                  err = pci_enable_device(dev);
>     74                  if (err)
>     75                          return err;
>     76                  if (dev_data)
>     77                          dev_data->enable_intx = 1;
>     78          } else if (pci_is_enabled(dev) && !is_enable_cmd(value)) {
>     79                  if (unlikely(verbose_request))
>     80                          printk(KERN_DEBUG DRV_NAME ": %s: disable\n",
>     81                                 pci_name(dev));
>     82                  pci_disable_device(dev);
>     83                  if (dev_data)
>     84                          dev_data->enable_intx = 0;
>     85          }
>     86
>     87          if (!dev->is_busmaster && is_master_cmd(value)) {
>     88                  if (unlikely(verbose_request))
>     89                          printk(KERN_DEBUG DRV_NAME ": %s: set bus master\n",
>     90                                 pci_name(dev));
>     91                  pci_set_master(dev);
>     92          } else if (dev->is_busmaster && !is_master_cmd(value)) {
>     93                  if (unlikely(verbose_request))
>     94                          printk(KERN_DEBUG DRV_NAME ": %s: clear bus master\n",
>     95                                 pci_name(dev));
>     96                  pci_clear_master(dev);
>     97          }
>     98
>     99          if (!(cmd->val & PCI_COMMAND_INVALIDATE) &&
>    100              (value & PCI_COMMAND_INVALIDATE)) {
>    101                  if (unlikely(verbose_request))
>    102                          printk(KERN_DEBUG
>    103                                 DRV_NAME ": %s: enable memory-write-invalidate\n",
>    104                                 pci_name(dev));
>    105                  err = pci_set_mwi(dev);
>    106                  if (err) {
>    107                          pr_warn("%s: cannot enable memory-write-invalidate (%d)\n",
>    108                                  pci_name(dev), err);
>    109                          value &= ~PCI_COMMAND_INVALIDATE;
>    110                  }
>    111          } else if ((cmd->val & PCI_COMMAND_INVALIDATE) &&
>    112                     !(value & PCI_COMMAND_INVALIDATE)) {
>    113                  if (unlikely(verbose_request))
>    114                          printk(KERN_DEBUG
>    115                                 DRV_NAME ": %s: disable memory-write-invalidate\n",
>    116                                 pci_name(dev));
>    117                  pci_clear_mwi(dev);
>    118          }
>    119
>    120          if (dev_data && dev_data->allow_interrupt_control) {
>  > 121                  if ((cmd->val ^ val) & PCI_COMMAND_INTX_DISABLE) {
>    122                          if (value & PCI_COMMAND_INTX_DISABLE) {
>    123                                  pci_intx(dev, 0);
>    124                          } else {
>    125                                  /* Do not allow enabling INTx together with MSI or MSI-X. */
>    126                                  switch (xen_pcibk_get_interrupt_type(dev)) {
>    127                                  case INTERRUPT_TYPE_NONE:
>    128                                  case INTERRUPT_TYPE_INTX:
>    129                                          pci_intx(dev, 1);
>    130                                          break;
>    131                                  default:
>    132                                          return PCIBIOS_SET_FAILED;
>    133                                  }
>    134                          }
>    135                  }
>    136          }
>    137
>    138          cmd->val = value;
>    139
>    140          if (!xen_pcibk_permissive && (!dev_data || !dev_data->permissive))
>    141                  return 0;
>    142
>    143          /* Only allow the guest to control certain bits. */
>    144          err = pci_read_config_word(dev, offset, &val);
>    145          if (err || val == value)
>    146                  return err;
>    147
>    148          value &= PCI_COMMAND_GUEST;
>    149          value |= val & ~PCI_COMMAND_GUEST;
>    150
>    151          return pci_write_config_word(dev, offset, value);
>    152  }
>    153
>
> ---
> 0-DAY kernel test infrastructure                 Open Source Technology Center
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/202001112351.gy4c3aUU%25lkp%40intel.com.



-- 
Thanks,
~Nick Desaulniers

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH v3] xen-pciback: optionally allow interrupt enable flag writes
  2020-01-13 21:25 ` Boris Ostrovsky
@ 2020-01-13 21:39   ` Marek Marczykowski-Górecki
  0 siblings, 0 replies; 4+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-13 21:39 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Juergen Gross, Stefano Stabellini, YueHaibing, open list,
	Simon Gaiser, Jan Beulich, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1058 bytes --]

On Mon, Jan 13, 2020 at 04:25:02PM -0500, Boris Ostrovsky wrote:
> 
> 
> On 1/10/20 10:43 PM, Marek Marczykowski-Górecki wrote:
> > @@ -117,6 +117,24 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
> >   		pci_clear_mwi(dev);
> >   	}
> > +	if (dev_data && dev_data->allow_interrupt_control) {
> > +		if ((cmd->val ^ val) & PCI_COMMAND_INTX_DISABLE) {
> > +			if (value & PCI_COMMAND_INTX_DISABLE) {
> > +				pci_intx(dev, 0);
> > +			} else {
> > +				/* Do not allow enabling INTx together with MSI or MSI-X. */
> > +				switch (xen_pcibk_get_interrupt_type(dev)) {
> > +				case INTERRUPT_TYPE_NONE:
> > +				case INTERRUPT_TYPE_INTX:
> > +					pci_intx(dev, 1);
> 
> If INTERRUPT_TYPE_INTX , why call pci_intx(1)?

Not needed indeed.

> 
> (I think I asked this last time as well).
> 
> 
> -boris
> 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Xen-devel] [PATCH v3] xen-pciback: optionally allow interrupt enable flag writes
  2020-01-11  3:43 Marek Marczykowski-Górecki
@ 2020-01-13 21:25 ` Boris Ostrovsky
  2020-01-13 21:39   ` Marek Marczykowski-Górecki
  0 siblings, 1 reply; 4+ messages in thread
From: Boris Ostrovsky @ 2020-01-13 21:25 UTC (permalink / raw)
  To: Marek Marczykowski-Górecki, xen-devel
  Cc: Juergen Gross, Stefano Stabellini, YueHaibing, open list,
	Simon Gaiser, Jan Beulich



On 1/10/20 10:43 PM, Marek Marczykowski-Górecki wrote:
> @@ -117,6 +117,24 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
>   		pci_clear_mwi(dev);
>   	}
>   
> +	if (dev_data && dev_data->allow_interrupt_control) {
> +		if ((cmd->val ^ val) & PCI_COMMAND_INTX_DISABLE) {
> +			if (value & PCI_COMMAND_INTX_DISABLE) {
> +				pci_intx(dev, 0);
> +			} else {
> +				/* Do not allow enabling INTx together with MSI or MSI-X. */
> +				switch (xen_pcibk_get_interrupt_type(dev)) {
> +				case INTERRUPT_TYPE_NONE:
> +				case INTERRUPT_TYPE_INTX:
> +					pci_intx(dev, 1);

If INTERRUPT_TYPE_INTX , why call pci_intx(1)?

(I think I asked this last time as well).


-boris



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Xen-devel] [PATCH v3] xen-pciback: optionally allow interrupt enable flag writes
@ 2020-01-11  3:43 Marek Marczykowski-Górecki
  2020-01-13 21:25 ` Boris Ostrovsky
  0 siblings, 1 reply; 4+ messages in thread
From: Marek Marczykowski-Górecki @ 2020-01-11  3:43 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Stefano Stabellini, YueHaibing,
	Marek Marczykowski-Górecki, open list, Simon Gaiser,
	Jan Beulich, Boris Ostrovsky

QEMU running in a stubdom needs to be able to set INTX_DISABLE, and the
MSI(-X) enable flags in the PCI config space. This adds an attribute
'allow_interrupt_control' which when set for a PCI device allows writes
to this flag(s). The toolstack will need to set this for stubdoms.
When enabled, guest (stubdomain) will be allowed to set relevant enable
flags, but only one at a time - i.e. it refuses to enable more than one
of INTx, MSI, MSI-X at a time.

This functionality is needed only for config space access done by device
model (stubdomain) serving a HVM with the actual PCI device. It is not
necessary and unsafe to enable direct access to those bits for PV domain
with the device attached. For PV domains, there are separate protocol
messages (XEN_PCI_OP_{enable,disable}_{msi,msix}) for this purpose.
Those ops in addition to setting enable bits, also configure MSI(-X) in
dom0 kernel - which is undesirable for PCI passthrough to HVM guests.

This should not introduce any new security issues since a malicious
guest (or stubdom) can already generate MSIs through other ways, see
[1] page 8. Additionally, when qemu runs in dom0, it already have direct
access to those bits.

This is the second iteration of this feature. First was proposed as a
direct Xen interface through a new hypercall, but ultimately it was
rejected by the maintainer, because of mixing pciback and hypercalls for
PCI config space access isn't a good design. Full discussion at [2].

[1]: https://invisiblethingslab.com/resources/2011/Software%20Attacks%20on%20Intel%20VT-d.pdf
[2]: https://xen.markmail.org/thread/smpgpws4umdzizze

[part of the commit message and sysfs handling]
Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
[the rest]
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Changes in v3:
 - return bitmap (or negative error) from
   xen_pcibk_get_interrupt_type(), to implicitly handle cases when
   multiple interrupt types are already enabled - disallow enabling in
   that case
 - add documentation
Changes in v2:
 - introduce xen_pcibk_get_interrupt_type() to deduplicate current
   INTx/MSI/MSI-X state check
 - fix checking MSI/MSI-X state on devices not supporting it

---
 .../ABI/testing/sysfs-driver-pciback          | 13 +++
 drivers/xen/xen-pciback/conf_space.c          | 36 ++++++++
 drivers/xen/xen-pciback/conf_space.h          |  7 ++
 .../xen/xen-pciback/conf_space_capability.c   | 88 +++++++++++++++++++
 drivers/xen/xen-pciback/conf_space_header.c   | 18 ++++
 drivers/xen/xen-pciback/pci_stub.c            | 66 ++++++++++++++
 drivers/xen/xen-pciback/pciback.h             |  1 +
 7 files changed, 229 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-pciback b/Documentation/ABI/testing/sysfs-driver-pciback
index 6a733bfa37e6..566a11f2c12f 100644
--- a/Documentation/ABI/testing/sysfs-driver-pciback
+++ b/Documentation/ABI/testing/sysfs-driver-pciback
@@ -11,3 +11,16 @@ Description:
                 #echo 00:19.0-E0:2:FF > /sys/bus/pci/drivers/pciback/quirks
                 will allow the guest to read and write to the configuration
                 register 0x0E.
+
+What:           /sys/bus/pci/drivers/pciback/allow_interrupt_control
+Date:           Jan 2020
+KernelVersion:  5.5
+Contact:        xen-devel@lists.xenproject.org
+Description:
+                List of devices which can have interrupt control flag (INTx,
+                MSI, MSI-X) set by a connected guest. It is meant to be set
+                only when the guest is a stubdomain hosting device model (qemu)
+                and the actual device is assigned to a HVM. It is not safe
+                (similar to permissive attribute) to set for a devices assigned
+                to a PV guest. The device is automatically removed from this
+                list when the connected pcifront terminates.
diff --git a/drivers/xen/xen-pciback/conf_space.c b/drivers/xen/xen-pciback/conf_space.c
index 60111719b01f..7697001e8ffc 100644
--- a/drivers/xen/xen-pciback/conf_space.c
+++ b/drivers/xen/xen-pciback/conf_space.c
@@ -286,6 +286,42 @@ int xen_pcibk_config_write(struct pci_dev *dev, int offset, int size, u32 value)
 	return xen_pcibios_err_to_errno(err);
 }
 
+int xen_pcibk_get_interrupt_type(struct pci_dev *dev)
+{
+	int err;
+	u16 val;
+	int ret = 0;
+
+	err = pci_read_config_word(dev, PCI_COMMAND, &val);
+	if (err)
+		return err;
+	if (!(val & PCI_COMMAND_INTX_DISABLE))
+		ret |= INTERRUPT_TYPE_INTX;
+
+	/* Do not trust dev->msi(x)_enabled here, as enabling could be done
+	 * bypassing the pci_*msi* functions, by the qemu.
+	 */
+	if (dev->msi_cap) {
+		err = pci_read_config_word(dev,
+				dev->msi_cap + PCI_MSI_FLAGS,
+				&val);
+		if (err)
+			return err;
+		if (val & PCI_MSI_FLAGS_ENABLE)
+			ret |= INTERRUPT_TYPE_MSI;
+	}
+	if (dev->msix_cap) {
+		err = pci_read_config_word(dev,
+				dev->msix_cap + PCI_MSIX_FLAGS,
+				&val);
+		if (err)
+			return err;
+		if (val & PCI_MSIX_FLAGS_ENABLE)
+			ret |= INTERRUPT_TYPE_MSIX;
+	}
+	return ret;
+}
+
 void xen_pcibk_config_free_dyn_fields(struct pci_dev *dev)
 {
 	struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(dev);
diff --git a/drivers/xen/xen-pciback/conf_space.h b/drivers/xen/xen-pciback/conf_space.h
index 22db630717ea..6ba6aa26dcee 100644
--- a/drivers/xen/xen-pciback/conf_space.h
+++ b/drivers/xen/xen-pciback/conf_space.h
@@ -65,6 +65,11 @@ struct config_field_entry {
 	void *data;
 };
 
+#define INTERRUPT_TYPE_NONE 0
+#define INTERRUPT_TYPE_INTX 1
+#define INTERRUPT_TYPE_MSI  2
+#define INTERRUPT_TYPE_MSIX 4
+
 extern bool xen_pcibk_permissive;
 
 #define OFFSET(cfg_entry) ((cfg_entry)->base_offset+(cfg_entry)->field->offset)
@@ -126,4 +131,6 @@ int xen_pcibk_config_capability_init(void);
 int xen_pcibk_config_header_add_fields(struct pci_dev *dev);
 int xen_pcibk_config_capability_add_fields(struct pci_dev *dev);
 
+int xen_pcibk_get_interrupt_type(struct pci_dev *dev);
+
 #endif				/* __XEN_PCIBACK_CONF_SPACE_H__ */
diff --git a/drivers/xen/xen-pciback/conf_space_capability.c b/drivers/xen/xen-pciback/conf_space_capability.c
index e5694133ebe5..d3a846119974 100644
--- a/drivers/xen/xen-pciback/conf_space_capability.c
+++ b/drivers/xen/xen-pciback/conf_space_capability.c
@@ -189,6 +189,84 @@ static const struct config_field caplist_pm[] = {
 	{}
 };
 
+static struct msi_msix_field_config {
+	u16 enable_bit; /* bit for enabling MSI/MSI-X */
+	int int_type; /* interrupt type for exclusiveness check */
+} msi_field_config = {
+	.enable_bit = PCI_MSI_FLAGS_ENABLE,
+	.int_type = INTERRUPT_TYPE_MSI,
+}, msix_field_config = {
+	.enable_bit = PCI_MSIX_FLAGS_ENABLE,
+	.int_type = INTERRUPT_TYPE_MSIX,
+};
+
+static void *msi_field_init(struct pci_dev *dev, int offset)
+{
+	return &msi_field_config;
+}
+
+static void *msix_field_init(struct pci_dev *dev, int offset)
+{
+	return &msix_field_config;
+}
+
+static int msi_msix_flags_write(struct pci_dev *dev, int offset, u16 new_value,
+				void *data)
+{
+	int err;
+	u16 old_value;
+	const struct msi_msix_field_config *field_config = data;
+	const struct xen_pcibk_dev_data *dev_data = pci_get_drvdata(dev);
+
+	if (xen_pcibk_permissive || dev_data->permissive)
+		goto write;
+
+	err = pci_read_config_word(dev, offset, &old_value);
+	if (err)
+		return err;
+
+	if (new_value == old_value)
+		return 0;
+
+	if (!dev_data->allow_interrupt_control ||
+	    (new_value ^ old_value) & ~field_config->enable_bit)
+		return PCIBIOS_SET_FAILED;
+
+	if (new_value & field_config->enable_bit) {
+		/* don't allow enabling together with other interrupt types */
+		int int_type = xen_pcibk_get_interrupt_type(dev);
+		if (int_type == INTERRUPT_TYPE_NONE ||
+		    int_type == field_config->int_type)
+			goto write;
+		return PCIBIOS_SET_FAILED;
+	}
+
+write:
+	return pci_write_config_word(dev, offset, new_value);
+}
+
+static const struct config_field caplist_msix[] = {
+	{
+		.offset    = PCI_MSIX_FLAGS,
+		.size      = 2,
+		.init      = msix_field_init,
+		.u.w.read  = xen_pcibk_read_config_word,
+		.u.w.write = msi_msix_flags_write,
+	},
+	{}
+};
+
+static const struct config_field caplist_msi[] = {
+	{
+		.offset    = PCI_MSI_FLAGS,
+		.size      = 2,
+		.init      = msi_field_init,
+		.u.w.read  = xen_pcibk_read_config_word,
+		.u.w.write = msi_msix_flags_write,
+	},
+	{}
+};
+
 static struct xen_pcibk_config_capability xen_pcibk_config_capability_pm = {
 	.capability = PCI_CAP_ID_PM,
 	.fields = caplist_pm,
@@ -197,11 +275,21 @@ static struct xen_pcibk_config_capability xen_pcibk_config_capability_vpd = {
 	.capability = PCI_CAP_ID_VPD,
 	.fields = caplist_vpd,
 };
+static struct xen_pcibk_config_capability xen_pcibk_config_capability_msi = {
+	.capability = PCI_CAP_ID_MSI,
+	.fields = caplist_msi,
+};
+static struct xen_pcibk_config_capability xen_pcibk_config_capability_msix = {
+	.capability = PCI_CAP_ID_MSIX,
+	.fields = caplist_msix,
+};
 
 int xen_pcibk_config_capability_init(void)
 {
 	register_capability(&xen_pcibk_config_capability_vpd);
 	register_capability(&xen_pcibk_config_capability_pm);
+	register_capability(&xen_pcibk_config_capability_msi);
+	register_capability(&xen_pcibk_config_capability_msix);
 
 	return 0;
 }
diff --git a/drivers/xen/xen-pciback/conf_space_header.c b/drivers/xen/xen-pciback/conf_space_header.c
index 10ae24b5a76e..ab60361d5a8a 100644
--- a/drivers/xen/xen-pciback/conf_space_header.c
+++ b/drivers/xen/xen-pciback/conf_space_header.c
@@ -117,6 +117,24 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
 		pci_clear_mwi(dev);
 	}
 
+	if (dev_data && dev_data->allow_interrupt_control) {
+		if ((cmd->val ^ val) & PCI_COMMAND_INTX_DISABLE) {
+			if (value & PCI_COMMAND_INTX_DISABLE) {
+				pci_intx(dev, 0);
+			} else {
+				/* Do not allow enabling INTx together with MSI or MSI-X. */
+				switch (xen_pcibk_get_interrupt_type(dev)) {
+				case INTERRUPT_TYPE_NONE:
+				case INTERRUPT_TYPE_INTX:
+					pci_intx(dev, 1);
+					break;
+				default:
+					return PCIBIOS_SET_FAILED;
+				}
+			}
+		}
+	}
+
 	cmd->val = value;
 
 	if (!xen_pcibk_permissive && (!dev_data || !dev_data->permissive))
diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
index 097410a7cdb7..7af93d65ed51 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -304,6 +304,8 @@ void pcistub_put_pci_dev(struct pci_dev *dev)
 	xen_pcibk_config_reset_dev(dev);
 	xen_pcibk_config_free_dyn_fields(dev);
 
+	dev_data->allow_interrupt_control = 0;
+
 	xen_unregister_device_domain_owner(dev);
 
 	spin_lock_irqsave(&found_psdev->lock, flags);
@@ -1431,6 +1433,65 @@ static ssize_t permissive_show(struct device_driver *drv, char *buf)
 }
 static DRIVER_ATTR_RW(permissive);
 
+static ssize_t allow_interrupt_control_store(struct device_driver *drv,
+					     const char *buf, size_t count)
+{
+	int domain, bus, slot, func;
+	int err;
+	struct pcistub_device *psdev;
+	struct xen_pcibk_dev_data *dev_data;
+
+	err = str_to_slot(buf, &domain, &bus, &slot, &func);
+	if (err)
+		goto out;
+
+	psdev = pcistub_device_find(domain, bus, slot, func);
+	if (!psdev) {
+		err = -ENODEV;
+		goto out;
+	}
+
+	dev_data = pci_get_drvdata(psdev->dev);
+	/* the driver data for a device should never be null at this point */
+	if (!dev_data) {
+		err = -ENXIO;
+		goto release;
+	}
+	dev_data->allow_interrupt_control = 1;
+release:
+	pcistub_device_put(psdev);
+out:
+	if (!err)
+		err = count;
+	return err;
+}
+
+static ssize_t allow_interrupt_control_show(struct device_driver *drv,
+					    char *buf)
+{
+	struct pcistub_device *psdev;
+	struct xen_pcibk_dev_data *dev_data;
+	size_t count = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pcistub_devices_lock, flags);
+	list_for_each_entry(psdev, &pcistub_devices, dev_list) {
+		if (count >= PAGE_SIZE)
+			break;
+		if (!psdev->dev)
+			continue;
+		dev_data = pci_get_drvdata(psdev->dev);
+		if (!dev_data || !dev_data->allow_interrupt_control)
+			continue;
+		count +=
+		    scnprintf(buf + count, PAGE_SIZE - count, "%s\n",
+			      pci_name(psdev->dev));
+	}
+	spin_unlock_irqrestore(&pcistub_devices_lock, flags);
+	return count;
+}
+static DRIVER_ATTR_RW(allow_interrupt_control);
+
 static void pcistub_exit(void)
 {
 	driver_remove_file(&xen_pcibk_pci_driver.driver, &driver_attr_new_slot);
@@ -1440,6 +1501,8 @@ static void pcistub_exit(void)
 	driver_remove_file(&xen_pcibk_pci_driver.driver, &driver_attr_quirks);
 	driver_remove_file(&xen_pcibk_pci_driver.driver,
 			   &driver_attr_permissive);
+	driver_remove_file(&xen_pcibk_pci_driver.driver,
+			   &driver_attr_allow_interrupt_control);
 	driver_remove_file(&xen_pcibk_pci_driver.driver,
 			   &driver_attr_irq_handlers);
 	driver_remove_file(&xen_pcibk_pci_driver.driver,
@@ -1530,6 +1593,9 @@ static int __init pcistub_init(void)
 	if (!err)
 		err = driver_create_file(&xen_pcibk_pci_driver.driver,
 					 &driver_attr_permissive);
+	if (!err)
+		err = driver_create_file(&xen_pcibk_pci_driver.driver,
+					 &driver_attr_allow_interrupt_control);
 
 	if (!err)
 		err = driver_create_file(&xen_pcibk_pci_driver.driver,
diff --git a/drivers/xen/xen-pciback/pciback.h b/drivers/xen/xen-pciback/pciback.h
index 263c059bff90..ce1077e32466 100644
--- a/drivers/xen/xen-pciback/pciback.h
+++ b/drivers/xen/xen-pciback/pciback.h
@@ -45,6 +45,7 @@ struct xen_pcibk_dev_data {
 	struct list_head config_fields;
 	struct pci_saved_state *pci_saved_state;
 	unsigned int permissive:1;
+	unsigned int allow_interrupt_control:1;
 	unsigned int warned_on_write:1;
 	unsigned int enable_intx:1;
 	unsigned int isr_on:1; /* Whether the IRQ handler is installed. */
-- 
2.21.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-13 21:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <202001112351.gy4c3aUU%lkp@intel.com>
2020-01-13 17:11 ` [Xen-devel] [PATCH v3] xen-pciback: optionally allow interrupt enable flag writes Nick Desaulniers
2020-01-11  3:43 Marek Marczykowski-Górecki
2020-01-13 21:25 ` Boris Ostrovsky
2020-01-13 21:39   ` Marek Marczykowski-Górecki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).