Re: [PATCH] PCI: vmd: Expose oob management window to users

2019-09-17 Thread Christoph Hellwig
On Mon, Sep 16, 2019 at 08:51:28AM -0600, Jon Derrick wrote:
> Some VMD devices provide a management window within MEMBAR2 that is used
> to communicate out-of-band with child devices. This patch creates a
> binary file for interacting with the interface.
> 
> OOB Reads/Writes are bounds-checked by sysfs_fs_bin_{read,write}

I don't think this is a reasonable interface.  If the OOB interfae
is useful please provide an actual kernel driver for it.


[PATCH] PCI: vmd: Expose oob management window to users

2019-09-16 Thread Jon Derrick
Some VMD devices provide a management window within MEMBAR2 that is used
to communicate out-of-band with child devices. This patch creates a
binary file for interacting with the interface.

OOB Reads/Writes are bounds-checked by sysfs_fs_bin_{read,write}

Signed-off-by: Jon Derrick 
---
Depends on 
https://lore.kernel.org/linux-pci/20190916135435.5017-1-jonathan.derr...@intel.com/T/#t

 drivers/pci/controller/vmd.c | 128 ---
 1 file changed, 117 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index a35d3f3996d7..b13954cf9c96 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -33,6 +33,8 @@
 
 #define MB2_SHADOW_OFFSET  0x2000
 #define MB2_SHADOW_SIZE16
+#define MB2_OOB_WINDOW_OFFSET  0x2010
+#define MB2_OOB_WINDOW_SIZE128
 
 enum vmd_features {
/*
@@ -47,6 +49,12 @@ enum vmd_features {
 * bus numbering
 */
VMD_FEAT_HAS_BUS_RESTRICTIONS   = (1 << 1),
+
+   /*
+* Device may provide an out-of-band management interface through a
+* read/write window
+*/
+   VMD_FEAT_HAS_OOB_WINDOW = (1 << 2),
 };
 
 /*
@@ -101,6 +109,10 @@ struct vmd_dev {
 
struct dma_map_ops  dma_ops;
struct dma_domain   dma_domain;
+
+   spinlock_t  oob_lock;
+   char __iomem*oob_addr;
+   struct bin_attribute*oob_attr;
 };
 
 static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
@@ -543,6 +555,68 @@ static void vmd_detach_resources(struct vmd_dev *vmd)
vmd->dev->resource[VMD_MEMBAR2].child = NULL;
 }
 
+static ssize_t vmd_oob_read(struct file *filp, struct kobject *kobj,
+   struct bin_attribute *attr, char *buf,
+   loff_t off, size_t count)
+{
+   struct vmd_dev *vmd = attr->private;
+   unsigned long flags;
+
+   if (!capable(CAP_SYS_ADMIN))
+   return -EACCES;
+
+   spin_lock_irqsave(&vmd->oob_lock, flags);
+   memcpy_fromio(&buf[off], &vmd->oob_addr[off], count);
+   spin_unlock_irqrestore(&vmd->oob_lock, flags);
+
+   return count;
+}
+
+static ssize_t vmd_oob_write(struct file *filp, struct kobject *kobj,
+struct bin_attribute *attr, char *buf,
+loff_t off, size_t count)
+{
+   struct vmd_dev *vmd = attr->private;
+   unsigned long flags;
+
+   if (!capable(CAP_SYS_ADMIN))
+   return -EACCES;
+
+   spin_lock_irqsave(&vmd->oob_lock, flags);
+   memcpy_toio(&vmd->oob_addr[off], &buf[off], count);
+   spin_unlock_irqrestore(&vmd->oob_lock, flags);
+
+   return count;
+}
+
+static int vmd_create_oob_file(struct vmd_dev *vmd)
+{
+   struct pci_dev *dev = vmd->dev;
+   struct bin_attribute *oob_attr;
+
+   oob_attr = devm_kzalloc(&vmd->dev->dev, sizeof(*oob_attr), GFP_ATOMIC);
+   if (!oob_attr)
+   return -ENOMEM;
+
+   spin_lock_init(&vmd->oob_lock);
+   sysfs_bin_attr_init(oob_attr);
+   vmd->oob_attr = oob_attr;
+   oob_attr->attr.name = "oob";
+   oob_attr->attr.mode = S_IRUSR | S_IWUSR;
+   oob_attr->size = MB2_OOB_WINDOW_SIZE;
+   oob_attr->read = vmd_oob_read;
+   oob_attr->write = vmd_oob_write;
+   oob_attr->private = (void *)vmd;
+
+   return sysfs_create_bin_file(&dev->dev.kobj, oob_attr);
+}
+
+static void vmd_destroy_oob_file(struct vmd_dev *vmd)
+{
+   if (vmd->oob_attr)
+   sysfs_remove_bin_file(&vmd->dev->dev.kobj, vmd->oob_attr);
+}
+
 /*
  * VMD domains start at 0x1 to not clash with ACPI _SEG domains.
  * Per ACPI r6.0, sec 6.5.6,  _SEG returns an integer, of which the lower
@@ -570,6 +644,7 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned 
long features)
resource_size_t offset[2] = {0};
resource_size_t membar2_offset = 0x2000;
struct pci_bus *child;
+   int ret;
 
/*
 * Shadow registers may exist in certain VMD device ids which allow
@@ -579,7 +654,6 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned 
long features)
 */
if (features & VMD_FEAT_HAS_MEMBAR_SHADOW) {
u32 vmlock;
-   int ret;
 
membar2_offset = MB2_SHADOW_OFFSET + MB2_SHADOW_SIZE;
ret = pci_read_config_dword(vmd->dev, PCI_REG_VMLOCK, &vmlock);
@@ -614,6 +688,24 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned 
long features)
vmd->busn_start = 128;
}
 
+   /*
+* Certain VMD devices provide a window for communicating with child
+* devices through a management interface
+*/
+   if (features & VMD_FEAT_HAS_OOB_WINDOW) {
+   membar2_offset = MB2_OOB_WINDOW_OFFSET + MB2_OOB_WINDOW_SIZE;
+   vmd->oob_addr = devm_ioremap(&vmd->dev->dev,
+