Re: [PATCH v5 0/8] s390: virtio: support protected virtualization
Sure. On 13.06.19 13:14, Halil Pasic wrote: On Thu, 13 Jun 2019 11:11:13 +0200 Michael Mueller wrote: Halil, I just ran my toleration tests successfully on current HW for this series. Michael Thanks Michael! May I add a Tested-by: Michael Mueller for each patch? On 12.06.19 13:12, Halil Pasic wrote: Enhanced virtualization protection technology may require the use of bounce buffers for I/O. While support for this was built into the virtio core, virtio-ccw wasn't changed accordingly. Some background on technology (not part of this series) and the terminology used. * Protected Virtualization (PV): Protected Virtualization guarantees, that non-shared memory of a guest that operates in PV mode private to that guest. I.e. any attempts by the hypervisor or other guests to access it will result in an exception. If supported by the environment (machine, KVM, guest VM) a guest can decide to change into PV mode by doing the appropriate ultravisor calls. * Ultravisor: A hardware/firmware entity that manages PV guests, and polices access to their memory. A PV guest prospect needs to interact with the ultravisor, to enter PV mode, and potentially to share pages (for I/O which should be encrypted by the guest). A guest interacts with the ultravisor via so called ultravisor calls. A hypervisor needs to interact with the ultravisor to facilitate interpretation, emulation and swapping. A hypervisor interacts with the ultravisor via ultravisor calls and via the SIE state description. Generally the ultravisor sanitizes hypervisor inputs so that the guest can not be corrupted (except for denial of service. What needs to be done = Thus what needs to be done to bring virtio-ccw up to speed with respect to protected virtualization is: * use some 'new' common virtio stuff * make sure that virtio-ccw specific stuff uses shared memory when talking to the hypervisor (except control/communication blocks like ORB, these are handled by the ultravisor) * make sure the DMA API does what is necessary to talk through shared memory if we are a protected virtualization guest. * make sure the common IO layer plays along as well (airqs, sense). Important notes * This patch set is based on Martins features branch (git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git branch 'features'). * Documentation is still very sketchy. I'm committed to improving this, but I'm currently hampered by some dependencies currently. * The existing naming in the common infrastructure (kernel internal interfaces) is pretty much based on the AMD SEV terminology. Thus the names aren't always perfect. There might be merit to changing these names to more abstract ones. I did not put much thought into that at the current stage. * Testing: Please use iommu_platform=on for any virtio devices you are going to test this code with (so virtio actually uses the DMA API). @Sebastian: I kept your r-b on patch 2 "s390/cio: introduce DMA pools to cio" despite the small changes pointed out below. Please do complain if it ain't OK for you! Change log == v4 --> v5: * work around dma_pool API not tolerating NULL dma pool (patch 4) * make the genpool based dma pools API tolerate NULL genpool (patch 2) * fix typo (patch 2) * fix unintended code move (patch 7) * add more r-b's v3 --> v4 * fixed cleanup in css_bus_init() (Connie) * made cio.h include genalloc.h instead of a forward declaration (Connie) * added comments about dma_mask/coherent_dma_mask values (Connie) * fixed error handling in virtio_ccw_init() (Connie) * got rid of the *vc_dma* wrappers (Connie) * added some Reviewed-bys * rebased on top of current master, no changes were necessary v2 --> v3: * patch 2/8 potential cio_dma_pool_init() returning NULL issue fixed potential cio_gp_dma_create() returning NULL issue fixed warning issues with doc type comments fixed unused define statement removed * patch 3/8 potential cio_gp_dma_create() returning NULL issue fixed whitespace issue fixed warning issues with doc type comments fixed * patch 8/8 potential cio_dma_zalloc() returning NULL issue fixed v1 --> v2: * patch "virtio/s390: use vring_create_virtqueue" went already upstream * patch "virtio/s390: DMA support for virtio-ccw" went already upstream * patch "virtio/s390: enable packed ring" went already upstream * Made dev.dma_mask point to dev.coherent_dma_mask for css, subchannel and ccw devices. * While rebasing 's390/airq: use DMA memory for adapter interrupts' the newly introduced kmem_cache was replaced with an equivalent dma_pool; the kalloc() allocations are now replaced with cio_dma_zalloc() allocations to avoid wasting almost a full page. * Made virtio-ccw use the new AIRQ_IV_CACHELINE flag. * fixed all remaining checkpatch issues RFC --> v1: * Fixed bugs fou
Re: [PATCH v5 0/8] s390: virtio: support protected virtualization
Halil, I just ran my toleration tests successfully on current HW for this series. Michael On 12.06.19 13:12, Halil Pasic wrote: Enhanced virtualization protection technology may require the use of bounce buffers for I/O. While support for this was built into the virtio core, virtio-ccw wasn't changed accordingly. Some background on technology (not part of this series) and the terminology used. * Protected Virtualization (PV): Protected Virtualization guarantees, that non-shared memory of a guest that operates in PV mode private to that guest. I.e. any attempts by the hypervisor or other guests to access it will result in an exception. If supported by the environment (machine, KVM, guest VM) a guest can decide to change into PV mode by doing the appropriate ultravisor calls. * Ultravisor: A hardware/firmware entity that manages PV guests, and polices access to their memory. A PV guest prospect needs to interact with the ultravisor, to enter PV mode, and potentially to share pages (for I/O which should be encrypted by the guest). A guest interacts with the ultravisor via so called ultravisor calls. A hypervisor needs to interact with the ultravisor to facilitate interpretation, emulation and swapping. A hypervisor interacts with the ultravisor via ultravisor calls and via the SIE state description. Generally the ultravisor sanitizes hypervisor inputs so that the guest can not be corrupted (except for denial of service. What needs to be done = Thus what needs to be done to bring virtio-ccw up to speed with respect to protected virtualization is: * use some 'new' common virtio stuff * make sure that virtio-ccw specific stuff uses shared memory when talking to the hypervisor (except control/communication blocks like ORB, these are handled by the ultravisor) * make sure the DMA API does what is necessary to talk through shared memory if we are a protected virtualization guest. * make sure the common IO layer plays along as well (airqs, sense). Important notes * This patch set is based on Martins features branch (git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git branch 'features'). * Documentation is still very sketchy. I'm committed to improving this, but I'm currently hampered by some dependencies currently. * The existing naming in the common infrastructure (kernel internal interfaces) is pretty much based on the AMD SEV terminology. Thus the names aren't always perfect. There might be merit to changing these names to more abstract ones. I did not put much thought into that at the current stage. * Testing: Please use iommu_platform=on for any virtio devices you are going to test this code with (so virtio actually uses the DMA API). @Sebastian: I kept your r-b on patch 2 "s390/cio: introduce DMA pools to cio" despite the small changes pointed out below. Please do complain if it ain't OK for you! Change log == v4 --> v5: * work around dma_pool API not tolerating NULL dma pool (patch 4) * make the genpool based dma pools API tolerate NULL genpool (patch 2) * fix typo (patch 2) * fix unintended code move (patch 7) * add more r-b's v3 --> v4 * fixed cleanup in css_bus_init() (Connie) * made cio.h include genalloc.h instead of a forward declaration (Connie) * added comments about dma_mask/coherent_dma_mask values (Connie) * fixed error handling in virtio_ccw_init() (Connie) * got rid of the *vc_dma* wrappers (Connie) * added some Reviewed-bys * rebased on top of current master, no changes were necessary v2 --> v3: * patch 2/8 potential cio_dma_pool_init() returning NULL issue fixed potential cio_gp_dma_create() returning NULL issue fixed warning issues with doc type comments fixed unused define statement removed * patch 3/8 potential cio_gp_dma_create() returning NULL issue fixed whitespace issue fixed warning issues with doc type comments fixed * patch 8/8 potential cio_dma_zalloc() returning NULL issue fixed v1 --> v2: * patch "virtio/s390: use vring_create_virtqueue" went already upstream * patch "virtio/s390: DMA support for virtio-ccw" went already upstream * patch "virtio/s390: enable packed ring" went already upstream * Made dev.dma_mask point to dev.coherent_dma_mask for css, subchannel and ccw devices. * While rebasing 's390/airq: use DMA memory for adapter interrupts' the newly introduced kmem_cache was replaced with an equivalent dma_pool; the kalloc() allocations are now replaced with cio_dma_zalloc() allocations to avoid wasting almost a full page. * Made virtio-ccw use the new AIRQ_IV_CACHELINE flag. * fixed all remaining checkpatch issues RFC --> v1: * Fixed bugs found by Connie (may_reduce and handling reduced, warning, split move -- thanks Connie!). * Fixed console bug found by Sebastian (thanks Sebastian!). * Removed the completely useless duplicate of dma-mapping.h spotted by Christoph (thanks Christoph!). * Don't use
Re: [PATCH v5 7/8] virtio/s390: use DMA memory for ccw I/O and classic notifiers
w_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_READ_STATUS);
/*
* If the channel program failed (should only happen if the device
* was hotunplugged, and then we clean up via the machine check
- * handler anyway), vcdev->status was not overwritten and we just
+ * handler anyway), vcdev->dma_area->status was not overwritten and we just
* return the old status, which is fine.
*/
- kfree(ccw);
+ ccw_device_dma_free(vcdev->cdev, ccw, sizeof(*ccw));
- return *vcdev->status;
+ return vcdev->dma_area->status;
}
static void virtio_ccw_set_status(struct virtio_device *vdev, u8 status)
{
struct virtio_ccw_device *vcdev = to_vc_device(vdev);
- u8 old_status = *vcdev->status;
+ u8 old_status = vcdev->dma_area->status;
struct ccw1 *ccw;
int ret;
- ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
+ ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));
if (!ccw)
return;
/* Write the status to the host. */
- *vcdev->status = status;
+ vcdev->dma_area->status = status;
ccw->cmd_code = CCW_CMD_WRITE_STATUS;
ccw->flags = 0;
ccw->count = sizeof(status);
- ccw->cda = (__u32)(unsigned long)vcdev->status;
+ ccw->cda = (__u32)(unsigned long)>dma_area->status;
ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_STATUS);
/* Write failed? We assume status is unchanged. */
if (ret)
- *vcdev->status = old_status;
- kfree(ccw);
+ vcdev->dma_area->status = old_status;
+ ccw_device_dma_free(vcdev->cdev, ccw, sizeof(*ccw));
}
static const char *virtio_ccw_bus_name(struct virtio_device *vdev)
@@ -1004,8 +1018,8 @@ static void virtio_ccw_release_dev(struct device *_d)
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_ccw_device *vcdev = to_vc_device(dev);
- kfree(vcdev->status);
- kfree(vcdev->config_block);
+ ccw_device_dma_free(vcdev->cdev, vcdev->dma_area,
+ sizeof(*vcdev->dma_area));
kfree(vcdev);
}
@@ -1213,12 +1227,12 @@ static int virtio_ccw_set_transport_rev(struct virtio_ccw_device *vcdev)
struct ccw1 *ccw;
int ret;
- ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
+ ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));
if (!ccw)
return -ENOMEM;
- rev = kzalloc(sizeof(*rev), GFP_DMA | GFP_KERNEL);
+ rev = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*rev));
if (!rev) {
- kfree(ccw);
+ ccw_device_dma_free(vcdev->cdev, ccw, sizeof(*ccw));
return -ENOMEM;
}
@@ -1248,8 +1262,8 @@ static int virtio_ccw_set_transport_rev(struct virtio_ccw_device *vcdev)
}
} while (ret == -EOPNOTSUPP);
- kfree(ccw);
- kfree(rev);
+ ccw_device_dma_free(vcdev->cdev, ccw, sizeof(*ccw));
+ ccw_device_dma_free(vcdev->cdev, rev, sizeof(*rev));
return ret;
}
@@ -1266,14 +1280,10 @@ static int virtio_ccw_online(struct ccw_device *cdev)
goto out_free;
}
vcdev->vdev.dev.parent = >dev;
- vcdev->config_block = kzalloc(sizeof(*vcdev->config_block),
- GFP_DMA | GFP_KERNEL);
- if (!vcdev->config_block) {
- ret = -ENOMEM;
- goto out_free;
- }
- vcdev->status = kzalloc(sizeof(*vcdev->status), GFP_DMA | GFP_KERNEL);
- if (!vcdev->status) {
+ vcdev->cdev = cdev;
+ vcdev->dma_area = ccw_device_dma_zalloc(vcdev->cdev,
+ sizeof(*vcdev->dma_area));
+ if (!vcdev->dma_area) {
ret = -ENOMEM;
goto out_free;
}
@@ -1282,7 +1292,6 @@ static int virtio_ccw_online(struct ccw_device *cdev)
vcdev->vdev.dev.release = virtio_ccw_release_dev;
vcdev->vdev.config = _ccw_config_ops;
- vcdev->cdev = cdev;
init_waitqueue_head(>wait_q);
INIT_LIST_HEAD(>virtqueues);
spin_lock_init(>lock);
@@ -1313,8 +1322,8 @@ static int virtio_ccw_online(struct ccw_device *cdev)
return ret;
out_free:
if (vcdev) {
- kfree(vcdev->status);
- kfree(vcdev->config_block);
+ ccw_device_dma_free(vcdev->cdev, vcdev->dma_area,
+ sizeof(*vcdev->dma_area));
}
kfree(vcdev);
return ret;
Reviewed-by: Michael Mueller
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 8/8] virtio/s390: make airq summary indicators DMA
On 12.06.19 13:12, Halil Pasic wrote:
Hypervisor needs to interact with the summary indicators, so these
The hypervisor...
need to be DMA memory as well (at least for protected virtualization
guests).
Signed-off-by: Halil Pasic
Reviewed-by: Cornelia Huck
---
drivers/s390/virtio/virtio_ccw.c | 32
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 800252955a2f..1a55e5942d36 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -140,11 +140,17 @@ static int virtio_ccw_use_airq = 1;
struct airq_info {
rwlock_t lock;
- u8 summary_indicator;
+ u8 summary_indicator_idx;
struct airq_struct airq;
struct airq_iv *aiv;
};
static struct airq_info *airq_areas[MAX_AIRQ_AREAS];
+static u8 *summary_indicators;
+
+static inline u8 *get_summary_indicator(struct airq_info *info)
+{
+ return summary_indicators + info->summary_indicator_idx;
+}
#define CCW_CMD_SET_VQ 0x13
#define CCW_CMD_VDEV_RESET 0x33
@@ -209,7 +215,7 @@ static void virtio_airq_handler(struct airq_struct *airq,
bool floating)
break;
vring_interrupt(0, (void *)airq_iv_get_ptr(info->aiv, ai));
}
- info->summary_indicator = 0;
+ *(get_summary_indicator(info)) = 0;
smp_wmb();
/* Walk through indicators field, summary indicator not active. */
for (ai = 0;;) {
@@ -221,7 +227,7 @@ static void virtio_airq_handler(struct airq_struct *airq,
bool floating)
read_unlock(>lock);
}
-static struct airq_info *new_airq_info(void)
+static struct airq_info *new_airq_info(int index)
{
struct airq_info *info;
int rc;
@@ -237,7 +243,8 @@ static struct airq_info *new_airq_info(void)
return NULL;
}
info->airq.handler = virtio_airq_handler;
- info->airq.lsi_ptr = >summary_indicator;
+ info->summary_indicator_idx = index;
+ info->airq.lsi_ptr = get_summary_indicator(info);
info->airq.lsi_mask = 0xff;
info->airq.isc = VIRTIO_AIRQ_ISC;
rc = register_adapter_interrupt(>airq);
@@ -259,7 +266,7 @@ static unsigned long get_airq_indicator(struct virtqueue
*vqs[], int nvqs,
for (i = 0; i < MAX_AIRQ_AREAS && !indicator_addr; i++) {
if (!airq_areas[i])
- airq_areas[i] = new_airq_info();
+ airq_areas[i] = new_airq_info(i);
info = airq_areas[i];
if (!info)
return 0;
@@ -345,7 +352,7 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
if (!thinint_area)
return;
thinint_area->summary_indicator =
- (unsigned long) _info->summary_indicator;
+ (unsigned long) get_summary_indicator(airq_info);
thinint_area->isc = VIRTIO_AIRQ_ISC;
ccw->cmd_code = CCW_CMD_SET_IND_ADAPTER;
ccw->count = sizeof(*thinint_area);
@@ -613,7 +620,7 @@ static int virtio_ccw_register_adapter_ind(struct
virtio_ccw_device *vcdev,
}
info = vcdev->airq_info;
thinint_area->summary_indicator =
- (unsigned long) >summary_indicator;
+ (unsigned long) get_summary_indicator(info);
thinint_area->isc = VIRTIO_AIRQ_ISC;
ccw->cmd_code = CCW_CMD_SET_IND_ADAPTER;
ccw->flags = CCW_FLAG_SLI;
@@ -1493,8 +1500,17 @@ static void __init no_auto_parse(void)
static int __init virtio_ccw_init(void)
{
+ int rc;
+
/* parse no_auto string before we do anything further */
no_auto_parse();
- return ccw_driver_register(_ccw_driver);
+
+ summary_indicators = cio_dma_zalloc(MAX_AIRQ_AREAS);
+ if (!summary_indicators)
+ return -ENOMEM;
+ rc = ccw_driver_register(_ccw_driver);
+ if (rc)
+ cio_dma_free(summary_indicators, MAX_AIRQ_AREAS);
+ return rc;
}
device_initcall(virtio_ccw_init);
Reviewed-by: Michael Mueller
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 6/8] virtio/s390: add indirection to indicators access
On 12.06.19 13:12, Halil Pasic wrote:
This will come in handy soon when we pull out the indicators from
virtio_ccw_device to a memory area that is shared with the hypervisor
(in particular for protected virtualization guests).
Signed-off-by: Halil Pasic
Reviewed-by: Pierre Morel
Reviewed-by: Cornelia Huck
---
drivers/s390/virtio/virtio_ccw.c | 40
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 1da7430f94c8..e96a8cc56ec2 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -68,6 +68,16 @@ struct virtio_ccw_device {
void *airq_info;
};
+static inline unsigned long *indicators(struct virtio_ccw_device *vcdev)
+{
+ return >indicators;
+}
+
+static inline unsigned long *indicators2(struct virtio_ccw_device *vcdev)
+{
+ return >indicators2;
+}
+
struct vq_info_block_legacy {
__u64 queue;
__u32 align;
@@ -338,17 +348,17 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
ccw->cda = (__u32)(unsigned long) thinint_area;
} else {
/* payload is the address of the indicators */
- indicatorp = kmalloc(sizeof(>indicators),
+ indicatorp = kmalloc(sizeof(indicators(vcdev)),
GFP_DMA | GFP_KERNEL);
if (!indicatorp)
return;
*indicatorp = 0;
ccw->cmd_code = CCW_CMD_SET_IND;
- ccw->count = sizeof(>indicators);
+ ccw->count = sizeof(indicators(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
}
/* Deregister indicators from host. */
- vcdev->indicators = 0;
+ *indicators(vcdev) = 0;
ccw->flags = 0;
ret = ccw_io_helper(vcdev, ccw,
vcdev->is_thinint ?
@@ -657,10 +667,10 @@ static int virtio_ccw_find_vqs(struct virtio_device
*vdev, unsigned nvqs,
* We need a data area under 2G to communicate. Our payload is
* the address of the indicators.
*/
- indicatorp = kmalloc(sizeof(>indicators), GFP_DMA | GFP_KERNEL);
+ indicatorp = kmalloc(sizeof(indicators(vcdev)), GFP_DMA | GFP_KERNEL);
if (!indicatorp)
goto out;
- *indicatorp = (unsigned long) >indicators;
+ *indicatorp = (unsigned long) indicators(vcdev);
if (vcdev->is_thinint) {
ret = virtio_ccw_register_adapter_ind(vcdev, vqs, nvqs, ccw);
if (ret)
@@ -669,21 +679,21 @@ static int virtio_ccw_find_vqs(struct virtio_device
*vdev, unsigned nvqs,
}
if (!vcdev->is_thinint) {
/* Register queue indicators with host. */
- vcdev->indicators = 0;
+ *indicators(vcdev) = 0;
ccw->cmd_code = CCW_CMD_SET_IND;
ccw->flags = 0;
- ccw->count = sizeof(>indicators);
+ ccw->count = sizeof(indicators(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_SET_IND);
if (ret)
goto out;
}
/* Register indicators2 with host for config changes */
- *indicatorp = (unsigned long) >indicators2;
- vcdev->indicators2 = 0;
+ *indicatorp = (unsigned long) indicators2(vcdev);
+ *indicators2(vcdev) = 0;
ccw->cmd_code = CCW_CMD_SET_CONF_IND;
ccw->flags = 0;
- ccw->count = sizeof(>indicators2);
+ ccw->count = sizeof(indicators2(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_SET_CONF_IND);
if (ret)
@@ -1093,17 +1103,17 @@ static void virtio_ccw_int_handler(struct ccw_device
*cdev,
vcdev->err = -EIO;
}
virtio_ccw_check_activity(vcdev, activity);
- for_each_set_bit(i, >indicators,
-sizeof(vcdev->indicators) * BITS_PER_BYTE) {
+ for_each_set_bit(i, indicators(vcdev),
+sizeof(*indicators(vcdev)) * BITS_PER_BYTE) {
/* The bit clear must happen before the vring kick. */
- clear_bit(i, >indicators);
+ clear_bit(i, indicators(vcdev));
barrier();
vq = virtio_ccw_vq_by_ind(vcdev, i);
vring_interrupt(0, vq);
}
- if (test_bit(0, >indicators2)) {
+ if (test_bit(0, indicators2(vcdev))) {
virtio_config_changed(>vdev);
- clear_bit(0, >indicators2);
+ clear_bit(0, indicators2(vcdev));
}
}
Reviewed-by: Michael Mueller
Michael
_
Re: [PATCH v5 5/8] virtio/s390: use cacheline aligned airq bit vectors
On 12.06.19 13:12, Halil Pasic wrote:
The flag AIRQ_IV_CACHELINE was recently added to airq_iv_create(). Let
us use it! We actually wanted the vector to span a cacheline all along.
Signed-off-by: Halil Pasic
Reviewed-by: Christian Borntraeger
Reviewed-by: Cornelia Huck
---
drivers/s390/virtio/virtio_ccw.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index f995798bb025..1da7430f94c8 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -216,7 +216,8 @@ static struct airq_info *new_airq_info(void)
if (!info)
return NULL;
rwlock_init(>lock);
- info->aiv = airq_iv_create(VIRTIO_IV_BITS, AIRQ_IV_ALLOC | AIRQ_IV_PTR);
+ info->aiv = airq_iv_create(VIRTIO_IV_BITS, AIRQ_IV_ALLOC | AIRQ_IV_PTR
+ | AIRQ_IV_CACHELINE);
if (!info->aiv) {
kfree(info);
return NULL;
Reviewed-by: Michael Mueller
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 4/8] s390/airq: use DMA memory for adapter interrupts
he", cio_get_dma_css_dev(),
+ cache_line_size(),
+ cache_line_size(), PAGE_SIZE);
if (!airq_iv_cache)
return -ENOMEM;
return 0;
}
-subsys_initcall(airq_init);
diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h
index 06a91743335a..4d6c7d16416e 100644
--- a/drivers/s390/cio/cio.h
+++ b/drivers/s390/cio/cio.h
@@ -135,6 +135,8 @@ extern int cio_commit_config(struct subchannel *sch);
int cio_tm_start_key(struct subchannel *sch, struct tcw *tcw, u8 lpm, u8 key);
int cio_tm_intrg(struct subchannel *sch);
+extern int __init airq_init(void);
+
/* Use with care. */
#ifdef CONFIG_CCW_CONSOLE
extern struct subchannel *cio_probe_console(void);
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index e0f19f1e82a0..1b867c941b86 100644
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -1184,6 +1184,7 @@ static int __init css_bus_init(void)
ret = cio_dma_pool_init();
if (ret)
goto out_unregister_pmn;
+ airq_init();
css_init_done = 1;
/* Enable default isc for I/O subchannels. */
Reviewed-by: Michael Mueller
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 3/8] s390/cio: add basic protected virtualization support
_device *cdev;/* pointer to the child ccw device */
struct {
unsigned int suspend:1; /* allow suspend */
unsigned int prefetch:1;/* deny prefetch */
unsigned int inter:1; /* suppress intermediate interrupts */
} __packed options;
+ struct io_subchannel_dma_area *dma_area;
+ dma_addr_t dma_area_dma;
} __aligned(8);
#define to_io_private(n) ((struct io_subchannel_private *) \
@@ -115,6 +120,13 @@ enum cdev_todo {
#define FAKE_CMD_IRB 1
#define FAKE_TM_IRB 2
+struct ccw_device_dma_area {
+ struct senseid senseid; /* SenseID info */
+ struct ccw1 iccws[2]; /* ccws for SNID/SID/SPGID commands */
+ struct irb irb; /* device status */
+ struct pgid pgid[8];/* path group IDs per chpid*/
+};
+
struct ccw_device_private {
struct ccw_device *cdev;
struct subchannel *sch;
@@ -156,11 +168,7 @@ struct ccw_device_private {
} __attribute__((packed)) flags;
unsigned long intparm; /* user interruption parameter */
struct qdio_irq *qdio_data;
- struct irb irb; /* device status */
int async_kill_io_rc;
- struct senseid senseid; /* SenseID info */
- struct pgid pgid[8];/* path group IDs per chpid*/
- struct ccw1 iccws[2]; /* ccws for SNID/SID/SPGID commands */
struct work_struct todo_work;
enum cdev_todo todo;
wait_queue_head_t wait_q;
@@ -169,6 +177,8 @@ struct ccw_device_private {
struct list_head cmb_list; /* list of measured devices */
u64 cmb_start_time; /* clock value of cmb reset */
void *cmb_wait; /* deferred cmb enable/disable */
+ struct gen_pool *dma_pool;
+ struct ccw_device_dma_area *dma_area;
enum interruption_class int_class;
};
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 6a3076881321..f995798bb025 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -66,7 +66,6 @@ struct virtio_ccw_device {
bool device_lost;
unsigned int config_ready;
void *airq_info;
- u64 dma_mask;
};
struct vq_info_block_legacy {
@@ -1255,16 +1254,7 @@ static int virtio_ccw_online(struct ccw_device *cdev)
ret = -ENOMEM;
goto out_free;
}
-
vcdev->vdev.dev.parent = >dev;
- cdev->dev.dma_mask = >dma_mask;
- /* we are fine with common virtio infrastructure using 64 bit DMA */
- ret = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64));
- if (ret) {
- dev_warn(>dev, "Failed to enable 64-bit DMA.\n");
- goto out_free;
- }
-
vcdev->config_block = kzalloc(sizeof(*vcdev->config_block),
GFP_DMA | GFP_KERNEL);
if (!vcdev->config_block) {
Reviewed-by: Michael Mueller
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 2/8] s390/cio: introduce DMA pools to cio
y(struct gen_pool *gp_dma, struct device *dma_dev)
+{
+ if (!gp_dma)
+ return;
+ /* this is quite ugly but no better idea */
+ gen_pool_for_each_chunk(gp_dma, __gp_dma_free_dma, dma_dev);
+ gen_pool_destroy(gp_dma);
+}
+
+static int cio_dma_pool_init(void)
+{
+ /* No need to free up the resources: compiled in */
+ cio_dma_pool = cio_gp_dma_create(cio_get_dma_css_dev(), 1);
+ if (!cio_dma_pool)
+ return -ENOMEM;
+ return 0;
+}
+
+void *cio_gp_dma_zalloc(struct gen_pool *gp_dma, struct device *dma_dev,
+ size_t size)
+{
+ dma_addr_t dma_addr;
+ unsigned long addr;
+ size_t chunk_size;
+
+ if (!cio_dma_pool)
+ return NULL;
+ addr = gen_pool_alloc(gp_dma, size);
+ while (!addr) {
+ chunk_size = round_up(size, PAGE_SIZE);
+ addr = (unsigned long) dma_alloc_coherent(dma_dev,
+chunk_size, _addr, CIO_DMA_GFP);
+ if (!addr)
+ return NULL;
+ gen_pool_add_virt(gp_dma, addr, dma_addr, chunk_size, -1);
+ addr = gen_pool_alloc(gp_dma, size);
+ }
+ return (void *) addr;
+}
+
+void cio_gp_dma_free(struct gen_pool *gp_dma, void *cpu_addr, size_t size)
+{
+ if (!cpu_addr)
+ return;
+ memset(cpu_addr, 0, size);
+ gen_pool_free(gp_dma, (unsigned long) cpu_addr, size);
+}
+
+/*
+ * Allocate dma memory from the css global pool. Intended for memory not
+ * specific to any single device within the css. The allocated memory
+ * is not guaranteed to be 31-bit addressable.
+ *
+ * Caution: Not suitable for early stuff like console.
+ */
+void *cio_dma_zalloc(size_t size)
+{
+ return cio_gp_dma_zalloc(cio_dma_pool, cio_get_dma_css_dev(), size);
+}
+
+void cio_dma_free(void *cpu_addr, size_t size)
+{
+ cio_gp_dma_free(cio_dma_pool, cpu_addr, size);
+}
+
/*
* Now that the driver core is running, we can setup our channel subsystem.
* The struct subchannel's are created during probing.
@@ -1059,16 +1179,21 @@ static int __init css_bus_init(void)
if (ret)
goto out_unregister;
ret = register_pm_notifier(_power_notifier);
- if (ret) {
- unregister_reboot_notifier(_reboot_notifier);
- goto out_unregister;
- }
+ if (ret)
+ goto out_unregister_rn;
+ ret = cio_dma_pool_init();
+ if (ret)
+ goto out_unregister_pmn;
css_init_done = 1;
/* Enable default isc for I/O subchannels. */
isc_register(IO_SCH_ISC);
return 0;
+out_unregister_pmn:
+ unregister_pm_notifier(_power_notifier);
+out_unregister_rn:
+ unregister_reboot_notifier(_reboot_notifier);
out_unregister:
while (i-- > 0) {
struct channel_subsystem *css = channel_subsystems[i];
Reviewed-by: Michael Mueller
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v5 1/8] s390/mm: force swiotlb for protected virtualization
On 12.06.19 13:12, Halil Pasic wrote:
On s390, protected virtualization guests have to use bounced I/O
buffers. That requires some plumbing.
sed 's/ / /'
Let us make sure, any device that uses DMA API with direct ops correctly
is spared from the problems, that a hypervisor attempting I/O to a
non-shared page would bring.
That sentence reads pretty cumbersome.
Signed-off-by: Halil Pasic
Reviewed-by: Claudio Imbrenda
---
arch/s390/Kconfig | 4 +++
arch/s390/include/asm/mem_encrypt.h | 18 +++
arch/s390/mm/init.c | 47 +
3 files changed, 69 insertions(+)
create mode 100644 arch/s390/include/asm/mem_encrypt.h
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 109243fdb6ec..88d8355b7bf7 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -1,4 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+config ARCH_HAS_MEM_ENCRYPT
+def_bool y
+
config MMU
def_bool y
@@ -187,6 +190,7 @@ config S390
select VIRT_CPU_ACCOUNTING
select ARCH_HAS_SCALED_CPUTIME
select HAVE_NMI
+ select SWIOTLB
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/s390/include/asm/mem_encrypt.h
b/arch/s390/include/asm/mem_encrypt.h
new file mode 100644
index ..0898c09a888c
--- /dev/null
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef S390_MEM_ENCRYPT_H__
+#define S390_MEM_ENCRYPT_H__
+
+#ifndef __ASSEMBLY__
+
+#define sme_me_mask0ULL
+
+static inline bool sme_active(void) { return false; }
+extern bool sev_active(void);
+
+int set_memory_encrypted(unsigned long addr, int numpages);
+int set_memory_decrypted(unsigned long addr, int numpages);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* S390_MEM_ENCRYPT_H__ */
+
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 14d1eae9fe43..f0bee6af3960 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -29,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -42,6 +44,8 @@
#include
#include
#include
+#include
+#include
pgd_t swapper_pg_dir[PTRS_PER_PGD] __section(.bss..swapper_pg_dir);
@@ -128,6 +132,47 @@ void mark_rodata_ro(void)
pr_info("Write protected read-only-after-init data: %luk\n", size >>
10);
}
+int set_memory_encrypted(unsigned long addr, int numpages)
+{
+ int i;
+
+ /* make specified pages unshared, (swiotlb, dma_free) */
+ for (i = 0; i < numpages; ++i) {
+ uv_remove_shared(addr);
+ addr += PAGE_SIZE;
+ }
+ return 0;
+}
+
+int set_memory_decrypted(unsigned long addr, int numpages)
+{
+ int i;
+ /* make specified pages shared (swiotlb, dma_alloca) */
+ for (i = 0; i < numpages; ++i) {
+ uv_set_shared(addr);
+ addr += PAGE_SIZE;
+ }
+ return 0;
+}
+
+/* are we a protected virtualization guest? */
+bool sev_active(void)
+{
+ return is_prot_virt_guest();
+}
+
+/* protected virtualization */
+static void pv_init(void)
+{
+ if (!is_prot_virt_guest())
+ return;
+
+ /* make sure bounce buffers are shared */
+ swiotlb_init(1);
+ swiotlb_update_mem_attributes();
+ swiotlb_force = SWIOTLB_FORCE;
+}
+
void __init mem_init(void)
{
cpumask_set_cpu(0, _mm.context.cpu_attach_mask);
@@ -136,6 +181,8 @@ void __init mem_init(void)
set_max_mapnr(max_low_pfn);
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
+ pv_init();
+
/* Setup guest page hinting */
cmma_init();
Reviewed-by: Michael Mueller
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 3/8] s390/cio: add basic protected virtualization support
On 03.06.19 15:42, Cornelia Huck wrote:
On Mon, 3 Jun 2019 14:45:03 +0200
Michael Mueller wrote:
On 03.06.19 14:06, Cornelia Huck wrote:
On Wed, 29 May 2019 14:26:52 +0200
Michael Mueller wrote:
@@ -1593,20 +1625,31 @@ struct ccw_device * __init
ccw_device_create_console(struct ccw_driver *drv)
return ERR_CAST(sch);
io_priv = kzalloc(sizeof(*io_priv), GFP_KERNEL | GFP_DMA);
- if (!io_priv) {
- put_device(>dev);
- return ERR_PTR(-ENOMEM);
- }
+ if (!io_priv)
+ goto err_priv;
+ io_priv->dma_area = dma_alloc_coherent(>dev,
+ sizeof(*io_priv->dma_area),
+ _priv->dma_area_dma, GFP_KERNEL);
+ if (!io_priv->dma_area)
+ goto err_dma_area;
set_io_private(sch, io_priv);
cdev = io_subchannel_create_ccwdev(sch);
if (IS_ERR(cdev)) {
put_device(>dev);
+ dma_free_coherent(>dev, sizeof(*io_priv->dma_area),
+ io_priv->dma_area, io_priv->dma_area_dma);
kfree(io_priv);
Shouldn't that branch do set_io_private(sch, NULL)? Not sure if any
code would make use of it, but it's probably better to clean out
references to freed objects.
Added behind kfree(). I hope nobody asks for a separate patch. ;)
I would probably have added it just before the kfree, but I'm not
asking for a separate patch ;)
I moved it.
return cdev;
}
cdev->drv = drv;
ccw_device_set_int_class(cdev);
return cdev;
+
+err_dma_area:
+ kfree(io_priv);
+err_priv:
+ put_device(>dev);
+ return ERR_PTR(-ENOMEM);
}
void __init ccw_device_destroy_console(struct ccw_device *cdev)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/8] s390/cio: introduce DMA pools to cio
On 03.06.19 15:34, Cornelia Huck wrote: On Mon, 3 Jun 2019 14:57:30 +0200 Halil Pasic wrote: On Mon, 3 Jun 2019 14:09:02 +0200 Michael Mueller wrote: @@ -224,6 +226,8 @@ struct subchannel *css_alloc_subchannel(struct subchannel_id schid, INIT_WORK(>todo_work, css_sch_todo); sch->dev.release = _subchannel_release; device_initialize(>dev); It might be helpful to add a comment why you use 31 bit here... @Halil, please let me know what comment you prefere here... How about? /* * The physical addresses of some the dma structures that * can belong to a subchannel need to fit 31 bit width (examples ccw,). */ "e.g. ccw"? + sch->dev.coherent_dma_mask = DMA_BIT_MASK(31); + sch->dev.dma_mask = >dev.coherent_dma_mask; return sch; err: @@ -899,6 +903,8 @@ static int __init setup_css(int nr) dev_set_name(>device, "css%x", nr); css->device.groups = cssdev_attr_groups; css->device.release = channel_subsystem_release; ...and 64 bit here. and here. /* * We currently allocate notifier bits with this (using css->device * as the device argument with the DMA API), and are fine with 64 bit * addresses. */ Thanks, that makes things hopefully clearer if we look at it some time in the future ;) Applied both with with requested change. Michael ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 3/8] s390/cio: add basic protected virtualization support
On 03.06.19 14:06, Cornelia Huck wrote:
On Wed, 29 May 2019 14:26:52 +0200
Michael Mueller wrote:
From: Halil Pasic
As virtio-ccw devices are channel devices, we need to use the dma area
for any communication with the hypervisor.
"we need to use the dma area within the common I/O layer for any
communication with the hypervisor. Note that we do not need to use that
area for control blocks directly referenced by instructions, e.g. the
orb."
using this now
...although I'm still not particularly confident about the actual
distinction here? I'm trusting you that you actually have tested it,
though :)
It handles neither QDIO in the common code, nor any device type specific
stuff (like channel programs constructed by the DASD driver).
An interesting side effect is that virtio structures are now going to
get allocated in 31 bit addressable storage.
Signed-off-by: Halil Pasic
Reviewed-by: Sebastian Ott
Signed-off-by: Michael Mueller
---
arch/s390/include/asm/ccwdev.h | 4 +++
drivers/s390/cio/ccwreq.c| 9 +++---
drivers/s390/cio/device.c| 67 +---
drivers/s390/cio/device_fsm.c| 49 +
drivers/s390/cio/device_id.c | 20 ++--
drivers/s390/cio/device_ops.c| 21 +++--
drivers/s390/cio/device_pgid.c | 22 +++--
drivers/s390/cio/device_status.c | 24 +++---
drivers/s390/cio/io_sch.h| 20 +---
drivers/s390/virtio/virtio_ccw.c | 10 --
10 files changed, 163 insertions(+), 83 deletions(-)
(...)
@@ -1593,20 +1625,31 @@ struct ccw_device * __init
ccw_device_create_console(struct ccw_driver *drv)
return ERR_CAST(sch);
io_priv = kzalloc(sizeof(*io_priv), GFP_KERNEL | GFP_DMA);
- if (!io_priv) {
- put_device(>dev);
- return ERR_PTR(-ENOMEM);
- }
+ if (!io_priv)
+ goto err_priv;
+ io_priv->dma_area = dma_alloc_coherent(>dev,
+ sizeof(*io_priv->dma_area),
+ _priv->dma_area_dma, GFP_KERNEL);
+ if (!io_priv->dma_area)
+ goto err_dma_area;
set_io_private(sch, io_priv);
cdev = io_subchannel_create_ccwdev(sch);
if (IS_ERR(cdev)) {
put_device(>dev);
+ dma_free_coherent(>dev, sizeof(*io_priv->dma_area),
+ io_priv->dma_area, io_priv->dma_area_dma);
kfree(io_priv);
Shouldn't that branch do set_io_private(sch, NULL)? Not sure if any
code would make use of it, but it's probably better to clean out
references to freed objects.
Added behind kfree(). I hope nobody asks for a separate patch. ;)
return cdev;
}
cdev->drv = drv;
ccw_device_set_int_class(cdev);
return cdev;
+
+err_dma_area:
+ kfree(io_priv);
+err_priv:
+ put_device(>dev);
+ return ERR_PTR(-ENOMEM);
}
void __init ccw_device_destroy_console(struct ccw_device *cdev)
With the reservations above,
Reviewed-by: Cornelia Huck
Thanks,
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/8] s390/cio: introduce DMA pools to cio
On 03.06.19 13:37, Cornelia Huck wrote:
On Wed, 29 May 2019 14:26:51 +0200
Michael Mueller wrote:
From: Halil Pasic
To support protected virtualization cio will need to make sure the
memory used for communication with the hypervisor is DMA memory.
Let us introduce one global pool for cio.
Our DMA pools are implemented as a gen_pool backed with DMA pages. The
idea is to avoid each allocation effectively wasting a page, as we
typically allocate much less than PAGE_SIZE.
Signed-off-by: Halil Pasic
Reviewed-by: Sebastian Ott
Signed-off-by: Michael Mueller
---
arch/s390/Kconfig | 1 +
arch/s390/include/asm/cio.h | 11
drivers/s390/cio/css.c | 120 ++--
3 files changed, 128 insertions(+), 4 deletions(-)
(...)
diff --git a/arch/s390/include/asm/cio.h b/arch/s390/include/asm/cio.h
index 1727180e8ca1..43c007d2775a 100644
--- a/arch/s390/include/asm/cio.h
+++ b/arch/s390/include/asm/cio.h
@@ -328,6 +328,17 @@ static inline u8 pathmask_to_pos(u8 mask)
void channel_subsystem_reinit(void);
extern void css_schedule_reprobe(void);
+extern void *cio_dma_zalloc(size_t size);
+extern void cio_dma_free(void *cpu_addr, size_t size);
+extern struct device *cio_get_dma_css_dev(void);
+
+struct gen_pool;
That forward declaration is a bit ugly... I guess the alternative was
include hell?
That's an easy one.
#include
+#include
#include
+void *cio_gp_dma_zalloc(struct gen_pool *gp_dma, struct device *dma_dev,
+ size_t size);
+void cio_gp_dma_free(struct gen_pool *gp_dma, void *cpu_addr, size_t size);
+void cio_gp_dma_destroy(struct gen_pool *gp_dma, struct device *dma_dev);
+struct gen_pool *cio_gp_dma_create(struct device *dma_dev, int nr_pages);
+
/* Function from drivers/s390/cio/chsc.c */
int chsc_sstpc(void *page, unsigned int op, u16 ctrl, u64 *clock_delta);
int chsc_sstpi(void *page, void *result, size_t size);
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index aea502922646..b97618497848 100644
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -20,6 +20,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -224,6 +226,8 @@ struct subchannel *css_alloc_subchannel(struct subchannel_id schid,
INIT_WORK(>todo_work, css_sch_todo);
sch->dev.release = _subchannel_release;
device_initialize(>dev);
It might be helpful to add a comment why you use 31 bit here...
@Halil, please let me know what comment you prefere here...
+ sch->dev.coherent_dma_mask = DMA_BIT_MASK(31);
+ sch->dev.dma_mask = >dev.coherent_dma_mask;
return sch;
err:
@@ -899,6 +903,8 @@ static int __init setup_css(int nr)
dev_set_name(>device, "css%x", nr);
css->device.groups = cssdev_attr_groups;
css->device.release = channel_subsystem_release;
...and 64 bit here.
and here.
+ css->device.coherent_dma_mask = DMA_BIT_MASK(64);
+ css->device.dma_mask = >device.coherent_dma_mask;
mutex_init(>mutex);
css->cssid = chsc_get_cssid(nr);
(...)
@@ -1059,16 +1168,19 @@ static int __init css_bus_init(void)
if (ret)
goto out_unregister;
ret = register_pm_notifier(_power_notifier);
- if (ret) {
- unregister_reboot_notifier(_reboot_notifier);
- goto out_unregister;
- }
+ if (ret)
+ goto out_unregister_rn;
+ ret = cio_dma_pool_init();
+ if (ret)
+ goto out_unregister_rn;
Don't you also need to unregister the pm notifier on failure here?
Mmh, that was the original intention. Thanks!
Other than that, I noticed only cosmetic issues; seems reasonable to me.
css_init_done = 1;
/* Enable default isc for I/O subchannels. */
isc_register(IO_SCH_ISC);
return 0;
+out_unregister_rn:
+ unregister_reboot_notifier(_reboot_notifier);
out_unregister:
while (i-- > 0) {
struct channel_subsystem *css = channel_subsystems[i];
Thanks,
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 3/8] s390/cio: add basic protected virtualization support
From: Halil Pasic
As virtio-ccw devices are channel devices, we need to use the dma area
for any communication with the hypervisor.
It handles neither QDIO in the common code, nor any device type specific
stuff (like channel programs constructed by the DASD driver).
An interesting side effect is that virtio structures are now going to
get allocated in 31 bit addressable storage.
Signed-off-by: Halil Pasic
Reviewed-by: Sebastian Ott
Signed-off-by: Michael Mueller
---
arch/s390/include/asm/ccwdev.h | 4 +++
drivers/s390/cio/ccwreq.c| 9 +++---
drivers/s390/cio/device.c| 67 +---
drivers/s390/cio/device_fsm.c| 49 +
drivers/s390/cio/device_id.c | 20 ++--
drivers/s390/cio/device_ops.c| 21 +++--
drivers/s390/cio/device_pgid.c | 22 +++--
drivers/s390/cio/device_status.c | 24 +++---
drivers/s390/cio/io_sch.h| 20 +---
drivers/s390/virtio/virtio_ccw.c | 10 --
10 files changed, 163 insertions(+), 83 deletions(-)
diff --git a/arch/s390/include/asm/ccwdev.h b/arch/s390/include/asm/ccwdev.h
index a29dd430fb40..865ce1cb86d5 100644
--- a/arch/s390/include/asm/ccwdev.h
+++ b/arch/s390/include/asm/ccwdev.h
@@ -226,6 +226,10 @@ extern int ccw_device_enable_console(struct ccw_device *);
extern void ccw_device_wait_idle(struct ccw_device *);
extern int ccw_device_force_console(struct ccw_device *);
+extern void *ccw_device_dma_zalloc(struct ccw_device *cdev, size_t size);
+extern void ccw_device_dma_free(struct ccw_device *cdev,
+ void *cpu_addr, size_t size);
+
int ccw_device_siosl(struct ccw_device *);
extern void ccw_device_get_schid(struct ccw_device *, struct subchannel_id *);
diff --git a/drivers/s390/cio/ccwreq.c b/drivers/s390/cio/ccwreq.c
index 603268a33ea1..73582a0a2622 100644
--- a/drivers/s390/cio/ccwreq.c
+++ b/drivers/s390/cio/ccwreq.c
@@ -63,7 +63,7 @@ static void ccwreq_stop(struct ccw_device *cdev, int rc)
return;
req->done = 1;
ccw_device_set_timeout(cdev, 0);
- memset(>private->irb, 0, sizeof(struct irb));
+ memset(>private->dma_area->irb, 0, sizeof(struct irb));
if (rc && rc != -ENODEV && req->drc)
rc = req->drc;
req->callback(cdev, req->data, rc);
@@ -86,7 +86,7 @@ static void ccwreq_do(struct ccw_device *cdev)
continue;
}
/* Perform start function. */
- memset(>private->irb, 0, sizeof(struct irb));
+ memset(>private->dma_area->irb, 0, sizeof(struct irb));
rc = cio_start(sch, cp, (u8) req->mask);
if (rc == 0) {
/* I/O started successfully. */
@@ -169,7 +169,7 @@ int ccw_request_cancel(struct ccw_device *cdev)
*/
static enum io_status ccwreq_status(struct ccw_device *cdev, struct irb *lcirb)
{
- struct irb *irb = >private->irb;
+ struct irb *irb = >private->dma_area->irb;
struct cmd_scsw *scsw = >scsw.cmd;
enum uc_todo todo;
@@ -187,7 +187,8 @@ static enum io_status ccwreq_status(struct ccw_device
*cdev, struct irb *lcirb)
CIO_TRACE_EVENT(2, "sensedata");
CIO_HEX_EVENT(2, >private->dev_id,
sizeof(struct ccw_dev_id));
- CIO_HEX_EVENT(2, >private->irb.ecw, SENSE_MAX_COUNT);
+ CIO_HEX_EVENT(2, >private->dma_area->irb.ecw,
+ SENSE_MAX_COUNT);
/* Check for command reject. */
if (irb->ecw[0] & SNS0_CMD_REJECT)
return IO_REJECTED;
diff --git a/drivers/s390/cio/device.c b/drivers/s390/cio/device.c
index 1540229a37bb..2830f4d7dfa1 100644
--- a/drivers/s390/cio/device.c
+++ b/drivers/s390/cio/device.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
@@ -687,6 +688,9 @@ ccw_device_release(struct device *dev)
struct ccw_device *cdev;
cdev = to_ccwdev(dev);
+ cio_gp_dma_free(cdev->private->dma_pool, cdev->private->dma_area,
+ sizeof(*cdev->private->dma_area));
+ cio_gp_dma_destroy(cdev->private->dma_pool, >dev);
/* Release reference of parent subchannel. */
put_device(cdev->dev.parent);
kfree(cdev->private);
@@ -696,15 +700,33 @@ ccw_device_release(struct device *dev)
static struct ccw_device * io_subchannel_allocate_dev(struct subchannel *sch)
{
struct ccw_device *cdev;
+ struct gen_pool *dma_pool;
cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
- if (cdev) {
- cdev->private = kzalloc(sizeof(struct ccw_device_private),
- GFP_KERNEL | GFP_DMA);
-
[PATCH v3 4/8] s390/airq: use DMA memory for adapter interrupts
From: Halil Pasic
Protected virtualization guests have to use shared pages for airq
notifier bit vectors, because hypervisor needs to write these bits.
Let us make sure we allocate DMA memory for the notifier bit vectors by
replacing the kmem_cache with a dma_cache and kalloc() with
cio_dma_zalloc().
Signed-off-by: Halil Pasic
Reviewed-by: Sebastian Ott
Signed-off-by: Michael Mueller
---
arch/s390/include/asm/airq.h | 2 ++
drivers/s390/cio/airq.c | 32
drivers/s390/cio/cio.h | 2 ++
drivers/s390/cio/css.c | 1 +
4 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h
index c10d2ee2dfda..01936fdfaddb 100644
--- a/arch/s390/include/asm/airq.h
+++ b/arch/s390/include/asm/airq.h
@@ -11,6 +11,7 @@
#define _ASM_S390_AIRQ_H
#include
+#include
struct airq_struct {
struct hlist_node list; /* Handler queueing. */
@@ -29,6 +30,7 @@ void unregister_adapter_interrupt(struct airq_struct *airq);
/* Adapter interrupt bit vector */
struct airq_iv {
unsigned long *vector; /* Adapter interrupt bit vector */
+ dma_addr_t vector_dma; /* Adapter interrupt bit vector dma */
unsigned long *avail; /* Allocation bit mask for the bit vector */
unsigned long *bitlock; /* Lock bit mask for the bit vector */
unsigned long *ptr; /* Pointer associated with each bit */
diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c
index 4534afc63591..89d26e43004d 100644
--- a/drivers/s390/cio/airq.c
+++ b/drivers/s390/cio/airq.c
@@ -16,9 +16,11 @@
#include
#include
#include
+#include
#include
#include
+#include
#include "cio.h"
#include "cio_debug.h"
@@ -27,7 +29,7 @@
static DEFINE_SPINLOCK(airq_lists_lock);
static struct hlist_head airq_lists[MAX_ISC+1];
-static struct kmem_cache *airq_iv_cache;
+static struct dma_pool *airq_iv_cache;
/**
* register_adapter_interrupt() - register adapter interrupt handler
@@ -115,6 +117,11 @@ void __init init_airq_interrupts(void)
setup_irq(THIN_INTERRUPT, _interrupt);
}
+static inline unsigned long iv_size(unsigned long bits)
+{
+ return BITS_TO_LONGS(bits) * sizeof(unsigned long);
+}
+
/**
* airq_iv_create - create an interrupt vector
* @bits: number of bits in the interrupt vector
@@ -132,17 +139,18 @@ struct airq_iv *airq_iv_create(unsigned long bits,
unsigned long flags)
goto out;
iv->bits = bits;
iv->flags = flags;
- size = BITS_TO_LONGS(bits) * sizeof(unsigned long);
+ size = iv_size(bits);
if (flags & AIRQ_IV_CACHELINE) {
if ((cache_line_size() * BITS_PER_BYTE) < bits)
goto out_free;
- iv->vector = kmem_cache_zalloc(airq_iv_cache, GFP_KERNEL);
+ iv->vector = dma_pool_zalloc(airq_iv_cache, GFP_KERNEL,
+>vector_dma);
if (!iv->vector)
goto out_free;
} else {
- iv->vector = kzalloc(size, GFP_KERNEL);
+ iv->vector = cio_dma_zalloc(size);
if (!iv->vector)
goto out_free;
}
@@ -179,9 +187,9 @@ struct airq_iv *airq_iv_create(unsigned long bits, unsigned
long flags)
kfree(iv->bitlock);
kfree(iv->avail);
if (iv->flags & AIRQ_IV_CACHELINE)
- kmem_cache_free(airq_iv_cache, iv->vector);
+ dma_pool_free(airq_iv_cache, iv->vector, iv->vector_dma);
else
- kfree(iv->vector);
+ cio_dma_free(iv->vector, size);
kfree(iv);
out:
return NULL;
@@ -198,9 +206,9 @@ void airq_iv_release(struct airq_iv *iv)
kfree(iv->ptr);
kfree(iv->bitlock);
if (iv->flags & AIRQ_IV_CACHELINE)
- kmem_cache_free(airq_iv_cache, iv->vector);
+ dma_pool_free(airq_iv_cache, iv->vector, iv->vector_dma);
else
- kfree(iv->vector);
+ cio_dma_free(iv->vector, iv_size(iv->bits));
kfree(iv->avail);
kfree(iv);
}
@@ -295,12 +303,12 @@ unsigned long airq_iv_scan(struct airq_iv *iv, unsigned
long start,
}
EXPORT_SYMBOL(airq_iv_scan);
-static int __init airq_init(void)
+int __init airq_init(void)
{
- airq_iv_cache = kmem_cache_create("airq_iv_cache", cache_line_size(),
- cache_line_size(), 0, NULL);
+ airq_iv_cache = dma_pool_create("airq_iv_cache", cio_get_dma_css_dev(),
+ cache_line_size(),
+ cache_line_size(), PAGE_SIZE);
if (!airq_iv_cache)
return -ENOMEM;
return 0;
}
-subsys_initcall(airq_init);
diff --git a/dri
[PATCH v3 2/8] s390/cio: introduce DMA pools to cio
From: Halil Pasic
To support protected virtualization cio will need to make sure the
memory used for communication with the hypervisor is DMA memory.
Let us introduce one global pool for cio.
Our DMA pools are implemented as a gen_pool backed with DMA pages. The
idea is to avoid each allocation effectively wasting a page, as we
typically allocate much less than PAGE_SIZE.
Signed-off-by: Halil Pasic
Reviewed-by: Sebastian Ott
Signed-off-by: Michael Mueller
---
arch/s390/Kconfig | 1 +
arch/s390/include/asm/cio.h | 11
drivers/s390/cio/css.c | 120 ++--
3 files changed, 128 insertions(+), 4 deletions(-)
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 88d8355b7bf7..2a245b56db8b 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -191,6 +191,7 @@ config S390
select ARCH_HAS_SCALED_CPUTIME
select HAVE_NMI
select SWIOTLB
+ select GENERIC_ALLOCATOR
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/s390/include/asm/cio.h b/arch/s390/include/asm/cio.h
index 1727180e8ca1..43c007d2775a 100644
--- a/arch/s390/include/asm/cio.h
+++ b/arch/s390/include/asm/cio.h
@@ -328,6 +328,17 @@ static inline u8 pathmask_to_pos(u8 mask)
void channel_subsystem_reinit(void);
extern void css_schedule_reprobe(void);
+extern void *cio_dma_zalloc(size_t size);
+extern void cio_dma_free(void *cpu_addr, size_t size);
+extern struct device *cio_get_dma_css_dev(void);
+
+struct gen_pool;
+void *cio_gp_dma_zalloc(struct gen_pool *gp_dma, struct device *dma_dev,
+ size_t size);
+void cio_gp_dma_free(struct gen_pool *gp_dma, void *cpu_addr, size_t size);
+void cio_gp_dma_destroy(struct gen_pool *gp_dma, struct device *dma_dev);
+struct gen_pool *cio_gp_dma_create(struct device *dma_dev, int nr_pages);
+
/* Function from drivers/s390/cio/chsc.c */
int chsc_sstpc(void *page, unsigned int op, u16 ctrl, u64 *clock_delta);
int chsc_sstpi(void *page, void *result, size_t size);
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index aea502922646..b97618497848 100644
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -20,6 +20,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -224,6 +226,8 @@ struct subchannel *css_alloc_subchannel(struct
subchannel_id schid,
INIT_WORK(>todo_work, css_sch_todo);
sch->dev.release = _subchannel_release;
device_initialize(>dev);
+ sch->dev.coherent_dma_mask = DMA_BIT_MASK(31);
+ sch->dev.dma_mask = >dev.coherent_dma_mask;
return sch;
err:
@@ -899,6 +903,8 @@ static int __init setup_css(int nr)
dev_set_name(>device, "css%x", nr);
css->device.groups = cssdev_attr_groups;
css->device.release = channel_subsystem_release;
+ css->device.coherent_dma_mask = DMA_BIT_MASK(64);
+ css->device.dma_mask = >device.coherent_dma_mask;
mutex_init(>mutex);
css->cssid = chsc_get_cssid(nr);
@@ -1018,6 +1024,109 @@ static struct notifier_block css_power_notifier = {
.notifier_call = css_power_event,
};
+#define CIO_DMA_GFP (GFP_KERNEL | __GFP_ZERO)
+static struct gen_pool *cio_dma_pool;
+
+/* Currently cio supports only a single css */
+struct device *cio_get_dma_css_dev(void)
+{
+ return _subsystems[0]->device;
+}
+
+struct gen_pool *cio_gp_dma_create(struct device *dma_dev, int nr_pages)
+{
+ struct gen_pool *gp_dma;
+ void *cpu_addr;
+ dma_addr_t dma_addr;
+ int i;
+
+ gp_dma = gen_pool_create(3, -1);
+ if (!gp_dma)
+ return NULL;
+ for (i = 0; i < nr_pages; ++i) {
+ cpu_addr = dma_alloc_coherent(dma_dev, PAGE_SIZE, _addr,
+ CIO_DMA_GFP);
+ if (!cpu_addr)
+ return gp_dma;
+ gen_pool_add_virt(gp_dma, (unsigned long) cpu_addr,
+ dma_addr, PAGE_SIZE, -1);
+ }
+ return gp_dma;
+}
+
+static void __gp_dma_free_dma(struct gen_pool *pool,
+ struct gen_pool_chunk *chunk, void *data)
+{
+ size_t chunk_size = chunk->end_addr - chunk->start_addr + 1;
+
+ dma_free_coherent((struct device *) data, chunk_size,
+(void *) chunk->start_addr,
+(dma_addr_t) chunk->phys_addr);
+}
+
+void cio_gp_dma_destroy(struct gen_pool *gp_dma, struct device *dma_dev)
+{
+ if (!gp_dma)
+ return;
+ /* this is qite ugly but no better idea */
+ gen_pool_for_each_chunk(gp_dma, __gp_dma_free_dma, dma_dev);
+ gen_pool_destroy(gp_dma);
+}
+
+static int cio_dma_pool_init(void)
+{
+ /* No need to free up the resources: compiled in */
+ cio_dma_pool = cio_gp_dma_create(cio_get_dma_css_dev(), 1);
+ if (!cio_dma_pool)
+ return -E
[PATCH v3 8/8] virtio/s390: make airq summary indicators DMA
From: Halil Pasic
Hypervisor needs to interact with the summary indicators, so these
need to be DMA memory as well (at least for protected virtualization
guests).
Signed-off-by: Halil Pasic
Signed-off-by: Michael Mueller
---
drivers/s390/virtio/virtio_ccw.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 03c9f7001fb1..efebd6dcd124 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -140,11 +140,17 @@ static int virtio_ccw_use_airq = 1;
struct airq_info {
rwlock_t lock;
- u8 summary_indicator;
+ u8 summary_indicator_idx;
struct airq_struct airq;
struct airq_iv *aiv;
};
static struct airq_info *airq_areas[MAX_AIRQ_AREAS];
+static u8 *summary_indicators;
+
+static inline u8 *get_summary_indicator(struct airq_info *info)
+{
+ return summary_indicators + info->summary_indicator_idx;
+}
#define CCW_CMD_SET_VQ 0x13
#define CCW_CMD_VDEV_RESET 0x33
@@ -225,7 +231,7 @@ static void virtio_airq_handler(struct airq_struct *airq,
bool floating)
break;
vring_interrupt(0, (void *)airq_iv_get_ptr(info->aiv, ai));
}
- info->summary_indicator = 0;
+ *(get_summary_indicator(info)) = 0;
smp_wmb();
/* Walk through indicators field, summary indicator not active. */
for (ai = 0;;) {
@@ -237,7 +243,7 @@ static void virtio_airq_handler(struct airq_struct *airq,
bool floating)
read_unlock(>lock);
}
-static struct airq_info *new_airq_info(void)
+static struct airq_info *new_airq_info(int index)
{
struct airq_info *info;
int rc;
@@ -253,7 +259,8 @@ static struct airq_info *new_airq_info(void)
return NULL;
}
info->airq.handler = virtio_airq_handler;
- info->airq.lsi_ptr = >summary_indicator;
+ info->summary_indicator_idx = index;
+ info->airq.lsi_ptr = get_summary_indicator(info);
info->airq.lsi_mask = 0xff;
info->airq.isc = VIRTIO_AIRQ_ISC;
rc = register_adapter_interrupt(>airq);
@@ -275,7 +282,7 @@ static unsigned long get_airq_indicator(struct virtqueue
*vqs[], int nvqs,
for (i = 0; i < MAX_AIRQ_AREAS && !indicator_addr; i++) {
if (!airq_areas[i])
- airq_areas[i] = new_airq_info();
+ airq_areas[i] = new_airq_info(i);
info = airq_areas[i];
if (!info)
return 0;
@@ -360,7 +367,7 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
if (!thinint_area)
return;
thinint_area->summary_indicator =
- (unsigned long) _info->summary_indicator;
+ (unsigned long) get_summary_indicator(airq_info);
thinint_area->isc = VIRTIO_AIRQ_ISC;
ccw->cmd_code = CCW_CMD_SET_IND_ADAPTER;
ccw->count = sizeof(*thinint_area);
@@ -625,7 +632,7 @@ static int virtio_ccw_register_adapter_ind(struct
virtio_ccw_device *vcdev,
}
info = vcdev->airq_info;
thinint_area->summary_indicator =
- (unsigned long) >summary_indicator;
+ (unsigned long) get_summary_indicator(info);
thinint_area->isc = VIRTIO_AIRQ_ISC;
ccw->cmd_code = CCW_CMD_SET_IND_ADAPTER;
ccw->flags = CCW_FLAG_SLI;
@@ -1501,6 +1508,11 @@ static int __init virtio_ccw_init(void)
{
/* parse no_auto string before we do anything further */
no_auto_parse();
+
+ summary_indicators = cio_dma_zalloc(MAX_AIRQ_AREAS);
+ if (!summary_indicators)
+ return -ENOMEM;
+
return ccw_driver_register(_ccw_driver);
}
device_initcall(virtio_ccw_init);
--
2.13.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 7/8] virtio/s390: use DMA memory for ccw I/O and classic notifiers
From: Halil Pasic
Before virtio-ccw could get away with not using DMA API for the pieces of
memory it does ccw I/O with. With protected virtualization this has to
change, since the hypervisor needs to read and sometimes also write these
pieces of memory.
The hypervisor is supposed to poke the classic notifiers, if these are
used, out of band with regards to ccw I/O. So these need to be allocated
as DMA memory (which is shared memory for protected virtualization
guests).
Let us factor out everything from struct virtio_ccw_device that needs to
be DMA memory in a satellite that is allocated as such.
Note: The control blocks of I/O instructions do not need to be shared.
These are marshalled by the ultravisor.
Signed-off-by: Halil Pasic
Reviewed-by: Pierre Morel
Signed-off-by: Michael Mueller
---
drivers/s390/virtio/virtio_ccw.c | 177 +--
1 file changed, 96 insertions(+), 81 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index e96a8cc56ec2..03c9f7001fb1 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -46,9 +46,15 @@ struct vq_config_block {
#define VIRTIO_CCW_CONFIG_SIZE 0x100
/* same as PCI config space size, should be enough for all drivers */
+struct vcdev_dma_area {
+ unsigned long indicators;
+ unsigned long indicators2;
+ struct vq_config_block config_block;
+ __u8 status;
+};
+
struct virtio_ccw_device {
struct virtio_device vdev;
- __u8 *status;
__u8 config[VIRTIO_CCW_CONFIG_SIZE];
struct ccw_device *cdev;
__u32 curr_io;
@@ -58,24 +64,22 @@ struct virtio_ccw_device {
spinlock_t lock;
struct mutex io_lock; /* Serializes I/O requests */
struct list_head virtqueues;
- unsigned long indicators;
- unsigned long indicators2;
- struct vq_config_block *config_block;
bool is_thinint;
bool going_away;
bool device_lost;
unsigned int config_ready;
void *airq_info;
+ struct vcdev_dma_area *dma_area;
};
static inline unsigned long *indicators(struct virtio_ccw_device *vcdev)
{
- return >indicators;
+ return >dma_area->indicators;
}
static inline unsigned long *indicators2(struct virtio_ccw_device *vcdev)
{
- return >indicators2;
+ return >dma_area->indicators2;
}
struct vq_info_block_legacy {
@@ -176,6 +180,22 @@ static struct virtio_ccw_device *to_vc_device(struct
virtio_device *vdev)
return container_of(vdev, struct virtio_ccw_device, vdev);
}
+static inline void *__vc_dma_alloc(struct virtio_device *vdev, size_t size)
+{
+ return ccw_device_dma_zalloc(to_vc_device(vdev)->cdev, size);
+}
+
+static inline void __vc_dma_free(struct virtio_device *vdev, size_t size,
+void *cpu_addr)
+{
+ return ccw_device_dma_free(to_vc_device(vdev)->cdev, cpu_addr, size);
+}
+
+#define vc_dma_alloc_struct(vdev, ptr) \
+ ({ptr = __vc_dma_alloc(vdev, sizeof(*(ptr))); })
+#define vc_dma_free_struct(vdev, ptr) \
+ __vc_dma_free(vdev, sizeof(*(ptr)), (ptr))
+
static void drop_airq_indicator(struct virtqueue *vq, struct airq_info *info)
{
unsigned long i, flags;
@@ -336,8 +356,7 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
struct airq_info *airq_info = vcdev->airq_info;
if (vcdev->is_thinint) {
- thinint_area = kzalloc(sizeof(*thinint_area),
- GFP_DMA | GFP_KERNEL);
+ vc_dma_alloc_struct(>vdev, thinint_area);
if (!thinint_area)
return;
thinint_area->summary_indicator =
@@ -348,8 +367,8 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
ccw->cda = (__u32)(unsigned long) thinint_area;
} else {
/* payload is the address of the indicators */
- indicatorp = kmalloc(sizeof(indicators(vcdev)),
-GFP_DMA | GFP_KERNEL);
+ indicatorp = __vc_dma_alloc(>vdev,
+ sizeof(indicators(vcdev)));
if (!indicatorp)
return;
*indicatorp = 0;
@@ -369,8 +388,9 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
"Failed to deregister indicators (%d)\n", ret);
else if (vcdev->is_thinint)
virtio_ccw_drop_indicators(vcdev);
- kfree(indicatorp);
- kfree(thinint_area);
+ __vc_dma_free(>vdev, sizeof(indicators(vcdev)),
+ indicatorp);
+ vc_dma_free_struct(>vdev, thinint_area);
}
static inline long __do_kvm_notify(struct subchannel_id schid,
@@ -417,15 +437,15 @@ static int virtio_ccw_read_vq_conf(struct
[PATCH v3 5/8] virtio/s390: use cacheline aligned airq bit vectors
From: Halil Pasic
The flag AIRQ_IV_CACHELINE was recently added to airq_iv_create(). Let
us use it! We actually wanted the vector to span a cacheline all along.
Signed-off-by: Halil Pasic
Signed-off-by: Michael Mueller
---
drivers/s390/virtio/virtio_ccw.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index f995798bb025..1da7430f94c8 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -216,7 +216,8 @@ static struct airq_info *new_airq_info(void)
if (!info)
return NULL;
rwlock_init(>lock);
- info->aiv = airq_iv_create(VIRTIO_IV_BITS, AIRQ_IV_ALLOC | AIRQ_IV_PTR);
+ info->aiv = airq_iv_create(VIRTIO_IV_BITS, AIRQ_IV_ALLOC | AIRQ_IV_PTR
+ | AIRQ_IV_CACHELINE);
if (!info->aiv) {
kfree(info);
return NULL;
--
2.13.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3 6/8] virtio/s390: add indirection to indicators access
From: Halil Pasic
This will come in handy soon when we pull out the indicators from
virtio_ccw_device to a memory area that is shared with the hypervisor
(in particular for protected virtualization guests).
Signed-off-by: Halil Pasic
Reviewed-by: Pierre Morel
Signed-off-by: Michael Mueller
---
drivers/s390/virtio/virtio_ccw.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 1da7430f94c8..e96a8cc56ec2 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -68,6 +68,16 @@ struct virtio_ccw_device {
void *airq_info;
};
+static inline unsigned long *indicators(struct virtio_ccw_device *vcdev)
+{
+ return >indicators;
+}
+
+static inline unsigned long *indicators2(struct virtio_ccw_device *vcdev)
+{
+ return >indicators2;
+}
+
struct vq_info_block_legacy {
__u64 queue;
__u32 align;
@@ -338,17 +348,17 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
ccw->cda = (__u32)(unsigned long) thinint_area;
} else {
/* payload is the address of the indicators */
- indicatorp = kmalloc(sizeof(>indicators),
+ indicatorp = kmalloc(sizeof(indicators(vcdev)),
GFP_DMA | GFP_KERNEL);
if (!indicatorp)
return;
*indicatorp = 0;
ccw->cmd_code = CCW_CMD_SET_IND;
- ccw->count = sizeof(>indicators);
+ ccw->count = sizeof(indicators(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
}
/* Deregister indicators from host. */
- vcdev->indicators = 0;
+ *indicators(vcdev) = 0;
ccw->flags = 0;
ret = ccw_io_helper(vcdev, ccw,
vcdev->is_thinint ?
@@ -657,10 +667,10 @@ static int virtio_ccw_find_vqs(struct virtio_device
*vdev, unsigned nvqs,
* We need a data area under 2G to communicate. Our payload is
* the address of the indicators.
*/
- indicatorp = kmalloc(sizeof(>indicators), GFP_DMA | GFP_KERNEL);
+ indicatorp = kmalloc(sizeof(indicators(vcdev)), GFP_DMA | GFP_KERNEL);
if (!indicatorp)
goto out;
- *indicatorp = (unsigned long) >indicators;
+ *indicatorp = (unsigned long) indicators(vcdev);
if (vcdev->is_thinint) {
ret = virtio_ccw_register_adapter_ind(vcdev, vqs, nvqs, ccw);
if (ret)
@@ -669,21 +679,21 @@ static int virtio_ccw_find_vqs(struct virtio_device
*vdev, unsigned nvqs,
}
if (!vcdev->is_thinint) {
/* Register queue indicators with host. */
- vcdev->indicators = 0;
+ *indicators(vcdev) = 0;
ccw->cmd_code = CCW_CMD_SET_IND;
ccw->flags = 0;
- ccw->count = sizeof(>indicators);
+ ccw->count = sizeof(indicators(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_SET_IND);
if (ret)
goto out;
}
/* Register indicators2 with host for config changes */
- *indicatorp = (unsigned long) >indicators2;
- vcdev->indicators2 = 0;
+ *indicatorp = (unsigned long) indicators2(vcdev);
+ *indicators2(vcdev) = 0;
ccw->cmd_code = CCW_CMD_SET_CONF_IND;
ccw->flags = 0;
- ccw->count = sizeof(>indicators2);
+ ccw->count = sizeof(indicators2(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_SET_CONF_IND);
if (ret)
@@ -1093,17 +1103,17 @@ static void virtio_ccw_int_handler(struct ccw_device
*cdev,
vcdev->err = -EIO;
}
virtio_ccw_check_activity(vcdev, activity);
- for_each_set_bit(i, >indicators,
-sizeof(vcdev->indicators) * BITS_PER_BYTE) {
+ for_each_set_bit(i, indicators(vcdev),
+sizeof(*indicators(vcdev)) * BITS_PER_BYTE) {
/* The bit clear must happen before the vring kick. */
- clear_bit(i, >indicators);
+ clear_bit(i, indicators(vcdev));
barrier();
vq = virtio_ccw_vq_by_ind(vcdev, i);
vring_interrupt(0, vq);
}
- if (test_bit(0, >indicators2)) {
+ if (test_bit(0, indicators2(vcdev))) {
virtio_config_changed(>vdev);
- clear_bit(0, >indicators2);
+ clear_bit(0, indicators2(vcdev));
}
}
--
2.13.4
___
Virtua
[PATCH v3 0/8] s390: virtio: support protected virtualization
Enhanced virtualization protection technology may require the use of bounce buffers for I/O. While support for this was built into the virtio core, virtio-ccw wasn't changed accordingly. Some background on technology (not part of this series) and the terminology used. * Protected Virtualization (PV): Protected Virtualization guarantees, that non-shared memory of a guest that operates in PV mode private to that guest. I.e. any attempts by the hypervisor or other guests to access it will result in an exception. If supported by the environment (machine, KVM, guest VM) a guest can decide to change into PV mode by doing the appropriate ultravisor calls. * Ultravisor: A hardware/firmware entity that manages PV guests, and polices access to their memory. A PV guest prospect needs to interact with the ultravisor, to enter PV mode, and potentially to share pages (for I/O which should be encrypted by the guest). A guest interacts with the ultravisor via so called ultravisor calls. A hypervisor needs to interact with the ultravisor to facilitate interpretation, emulation and swapping. A hypervisor interacts with the ultravisor via ultravisor calls and via the SIE state description. Generally the ultravisor sanitizes hypervisor inputs so that the guest can not be corrupted (except for denial of service. What needs to be done = Thus what needs to be done to bring virtio-ccw up to speed with respect to protected virtualization is: * use some 'new' common virtio stuff * make sure that virtio-ccw specific stuff uses shared memory when talking to the hypervisor (except control/communication blocks like ORB, these are handled by the ultravisor) * make sure the DMA API does what is necessary to talk through shared memory if we are a protected virtualization guest. * make sure the common IO layer plays along as well (airqs, sense). Important notes * This patch set is based on Martins features branch (git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git branch 'features'). * Documentation is still very sketchy. I'm committed to improving this, but I'm currently hampered by some dependencies currently. * The existing naming in the common infrastructure (kernel internal interfaces) is pretty much based on the AMD SEV terminology. Thus the names aren't always perfect. There might be merit to changing these names to more abstract ones. I did not put much thought into that at the current stage. * Testing: Please use iommu_platform=on for any virtio devices you are going to test this code with (so virtio actually uses the DMA API). Change log == v2 --> v3: * patch 2/8 potential cio_dma_pool_init() returning NULL issue fixed potential cio_gp_dma_create() returning NULL issue fixed warning issues with doc type comments fixed unused define statement removed * patch 3/8 potential cio_gp_dma_create() returning NULL issue fixed whitespace issue fixed warning issues with doc type comments fixed * patch 8/8 potential cio_dma_zalloc() returning NULL issue fixed v1 --> v2: * patch "virtio/s390: use vring_create_virtqueue" went already upstream * patch "virtio/s390: DMA support for virtio-ccw" went already upstream * patch "virtio/s390: enable packed ring" went already upstream * Made dev.dma_mask point to dev.coherent_dma_mask for css, subchannel and ccw devices. * While rebasing 's390/airq: use DMA memory for adapter interrupts' the newly introduced kmem_cache was replaced with an equivalent dma_pool; the kalloc() allocations are now replaced with cio_dma_zalloc() allocations to avoid wasting almost a full page. * Made virtio-ccw use the new AIRQ_IV_CACHELINE flag. * fixed all remaining checkpatch issues RFC --> v1: * Fixed bugs found by Connie (may_reduce and handling reduced, warning, split move -- thanks Connie!). * Fixed console bug found by Sebastian (thanks Sebastian!). * Removed the completely useless duplicate of dma-mapping.h spotted by Christoph (thanks Christoph!). * Don't use the global DMA pool for subchannel and ccw device owned memory as requested by Sebastian. Consequences: * Both subchannel and ccw devices have their dma masks now (both specifying 31 bit addressable) * We require at least 2 DMA pages per ccw device now, most of this memory is wasted though. * DMA memory allocated by virtio is also 31 bit addressable now as virtio uses the parent (which is the ccw device). * Enabled packed ring. * Rebased onto Martins feature branch; using the actual uv (ultravisor) interface instead of TODO comments. * Added some explanations to the cover letter (Connie, David). * Squashed a couple of patches together and fixed some text stuff. Halil Pasic (8): s390/mm: force swiotlb for protected virtualization s390/cio: introduce DMA pools to cio s390/cio: add basic protected virtualization support s390/airq: use DMA memory for adapter
[PATCH v3 1/8] s390/mm: force swiotlb for protected virtualization
From: Halil Pasic
On s390, protected virtualization guests have to use bounced I/O
buffers. That requires some plumbing.
Let us make sure, any device that uses DMA API with direct ops correctly
is spared from the problems, that a hypervisor attempting I/O to a
non-shared page would bring.
Signed-off-by: Halil Pasic
Reviewed-by: Claudio Imbrenda
Signed-off-by: Michael Mueller
---
arch/s390/Kconfig | 4
arch/s390/include/asm/mem_encrypt.h | 18 ++
arch/s390/mm/init.c | 47 +
3 files changed, 69 insertions(+)
create mode 100644 arch/s390/include/asm/mem_encrypt.h
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 109243fdb6ec..88d8355b7bf7 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -1,4 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+config ARCH_HAS_MEM_ENCRYPT
+def_bool y
+
config MMU
def_bool y
@@ -187,6 +190,7 @@ config S390
select VIRT_CPU_ACCOUNTING
select ARCH_HAS_SCALED_CPUTIME
select HAVE_NMI
+ select SWIOTLB
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/s390/include/asm/mem_encrypt.h
b/arch/s390/include/asm/mem_encrypt.h
new file mode 100644
index ..0898c09a888c
--- /dev/null
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef S390_MEM_ENCRYPT_H__
+#define S390_MEM_ENCRYPT_H__
+
+#ifndef __ASSEMBLY__
+
+#define sme_me_mask0ULL
+
+static inline bool sme_active(void) { return false; }
+extern bool sev_active(void);
+
+int set_memory_encrypted(unsigned long addr, int numpages);
+int set_memory_decrypted(unsigned long addr, int numpages);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* S390_MEM_ENCRYPT_H__ */
+
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 14d1eae9fe43..f0bee6af3960 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -29,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -42,6 +44,8 @@
#include
#include
#include
+#include
+#include
pgd_t swapper_pg_dir[PTRS_PER_PGD] __section(.bss..swapper_pg_dir);
@@ -128,6 +132,47 @@ void mark_rodata_ro(void)
pr_info("Write protected read-only-after-init data: %luk\n", size >>
10);
}
+int set_memory_encrypted(unsigned long addr, int numpages)
+{
+ int i;
+
+ /* make specified pages unshared, (swiotlb, dma_free) */
+ for (i = 0; i < numpages; ++i) {
+ uv_remove_shared(addr);
+ addr += PAGE_SIZE;
+ }
+ return 0;
+}
+
+int set_memory_decrypted(unsigned long addr, int numpages)
+{
+ int i;
+ /* make specified pages shared (swiotlb, dma_alloca) */
+ for (i = 0; i < numpages; ++i) {
+ uv_set_shared(addr);
+ addr += PAGE_SIZE;
+ }
+ return 0;
+}
+
+/* are we a protected virtualization guest? */
+bool sev_active(void)
+{
+ return is_prot_virt_guest();
+}
+
+/* protected virtualization */
+static void pv_init(void)
+{
+ if (!is_prot_virt_guest())
+ return;
+
+ /* make sure bounce buffers are shared */
+ swiotlb_init(1);
+ swiotlb_update_mem_attributes();
+ swiotlb_force = SWIOTLB_FORCE;
+}
+
void __init mem_init(void)
{
cpumask_set_cpu(0, _mm.context.cpu_attach_mask);
@@ -136,6 +181,8 @@ void __init mem_init(void)
set_max_mapnr(max_low_pfn);
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
+ pv_init();
+
/* Setup guest page hinting */
cmma_init();
--
2.13.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 3/8] s390/cio: add basic protected virtualization support
On 27.05.19 15:31, Cornelia Huck wrote:
On Mon, 27 May 2019 14:30:14 +0200
Halil Pasic wrote:
On Mon, 27 May 2019 12:38:02 +0200
Cornelia Huck wrote:
On Thu, 23 May 2019 18:22:04 +0200
Michael Mueller wrote:
From: Halil Pasic
As virtio-ccw devices are channel devices, we need to use the dma area
for any communication with the hypervisor.
It handles neither QDIO in the common code, nor any device type specific
stuff (like channel programs constructed by the DASD driver).
An interesting side effect is that virtio structures are now going to
get allocated in 31 bit addressable storage.
Signed-off-by: Halil Pasic
[Side note: you really should add your s-o-b if you send someone else's
patches... if Halil ends up committing them, it's fine, though.]
---
arch/s390/include/asm/ccwdev.h | 4 +++
drivers/s390/cio/ccwreq.c| 9 +++---
drivers/s390/cio/device.c| 64 +---
drivers/s390/cio/device_fsm.c| 53 -
drivers/s390/cio/device_id.c | 20 +++--
drivers/s390/cio/device_ops.c| 21 +++--
drivers/s390/cio/device_pgid.c | 22 +++---
drivers/s390/cio/device_status.c | 24 +++
drivers/s390/cio/io_sch.h| 20 +
drivers/s390/virtio/virtio_ccw.c | 10 ---
10 files changed, 164 insertions(+), 83 deletions(-)
(...)
@@ -1593,20 +1622,31 @@ struct ccw_device * __init
ccw_device_create_console(struct ccw_driver *drv)
return ERR_CAST(sch);
io_priv = kzalloc(sizeof(*io_priv), GFP_KERNEL | GFP_DMA);
- if (!io_priv) {
- put_device(>dev);
- return ERR_PTR(-ENOMEM);
- }
+ if (!io_priv)
+ goto err_priv;
+ io_priv->dma_area = dma_alloc_coherent(>dev,
+ sizeof(*io_priv->dma_area),
+ _priv->dma_area_dma, GFP_KERNEL);
Even though we'll only end up here for 3215 or 3270 consoles, this sent
me looking.
This code is invoked via console_init(). A few lines down in
start_kernel(), we have
/*
* This needs to be called before any devices perform DMA
* operations that might use the SWIOTLB bounce buffers. It will
* mark the bounce buffers as decrypted so that their usage will
* not cause "plain-text" data to be decrypted when accessed.
*/
mem_encrypt_init();
So, I'm wondering if creating the console device interacts in any way
with the memory encryption interface?
I do things a bit different than x86: the SWIOTLB stuff is set up in
mem_init(). So I think we should be fine. If there is a down-side to
calling swiotlb_update_mem_attributes() earlier, honestly I'm
not sure.
Neither am I; do any of the folks who looked at the swiotlb patch have
an idea?
[Does basic recognition work if you start a protected virt guest with a
3270 console? I realize that the console is unlikely to work, but that
should at least exercise this code path.]
I've already had some thoughts along these lines and slapped
-device x-terminal3270,chardev=char_0,devno=fe.0.000a,id=terminal_0 \
on my qemu command line. The ccw device does show up in the guest...
Device Subchan. DevType CU Type Use PIM PAM POM CHPIDs
--
0.0. 0.0. /00 3832/01 yes 80 80 ff
0.0.000a 0.0.0001 /00 3270/00 80 80 ff 0100
0.0.0002 0.0.0002 /00 3832/09 yes 80 80 ff
0.0.0300 0.0.0003 /00 3832/02 yes 80 80 ff
0.0.0301 0.0.0004 /00 3832/02 yes 80 80 ff
But I would not call it a comprehensive test...
If you only add the device, it will show up as a normal ccw device in
the guest; i.e. device recognition is done at the same time as for the
other ccw devices. Still good to see that nothing breaks there :)
To actually make the guest use the 3270 as its console, I guess you
need to explicitly force it (see
https://wiki.qemu.org/Features/3270#Using_3270_as_the_console)...
actually starting the console will almost certainly fail; but you can
at least check whether device recognition in the console path works.
Mimu, do we have something more elaborate with regards to this?
I ran that with success
[root@ap01 ~]# lscss | grep 3270
0.0.002a 0.0.0008 /00 3270/00 yes 80 80 ff 0100
and was able to connect and login.
Michael
I don't think we need extensive testing here; just checking that the
sequence is not fundamentally broken.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 6/8] virtio/s390: add indirection to indicators access
On 27.05.19 14:10, Cornelia Huck wrote:
On Mon, 27 May 2019 13:57:06 +0200
Halil Pasic wrote:
On Mon, 27 May 2019 13:00:28 +0200
Cornelia Huck wrote:
On Thu, 23 May 2019 18:22:07 +0200
Michael Mueller wrote:
From: Halil Pasic
This will come in handy soon when we pull out the indicators from
virtio_ccw_device to a memory area that is shared with the hypervisor
(in particular for protected virtualization guests).
Signed-off-by: Halil Pasic
Reviewed-by: Pierre Morel
---
drivers/s390/virtio/virtio_ccw.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
@@ -338,17 +348,17 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
ccw->cda = (__u32)(unsigned long) thinint_area;
} else {
/* payload is the address of the indicators */
- indicatorp = kmalloc(sizeof(>indicators),
+ indicatorp = kmalloc(sizeof(indicators(vcdev)),
GFP_DMA | GFP_KERNEL);
if (!indicatorp)
return;
*indicatorp = 0;
ccw->cmd_code = CCW_CMD_SET_IND;
- ccw->count = sizeof(>indicators);
+ ccw->count = sizeof(indicators(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
}
/* Deregister indicators from host. */
- vcdev->indicators = 0;
+ *indicators(vcdev) = 0;
I'm not too hot about this notation, but it's not wrong and a minor
thing :)
I don't have any better ideas :/
ccw->flags = 0;
ret = ccw_io_helper(vcdev, ccw,
vcdev->is_thinint ?
Patch looks reasonable and not dependent on the other patches here.
looks reasonable == r-b?
Not dependent in a sense that this patch could be made a first patch in
the series. A subsequent patch depends on it.
What is the plan with these patches? I can either pick patch 5+6 and
let them go through the virtio tree, or give my r-b and let them go
through the s390 tree. The former is probably the quicker route, but
the latter has less potential for dependency issues.
please give your r-b then for these.
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 8/8] virtio/s390: make airq summary indicators DMA
On 28.05.19 16:33, Halil Pasic wrote:
On Mon, 27 May 2019 14:00:18 +0200
Cornelia Huck wrote:
On Thu, 23 May 2019 18:22:09 +0200
Michael Mueller wrote:
From: Halil Pasic
Hypervisor needs to interact with the summary indicators, so these
need to be DMA memory as well (at least for protected virtualization
guests).
Signed-off-by: Halil Pasic
---
drivers/s390/virtio/virtio_ccw.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
(...)
@@ -1501,6 +1508,7 @@ static int __init virtio_ccw_init(void)
{
/* parse no_auto string before we do anything further */
no_auto_parse();
+ summary_indicators = cio_dma_zalloc(MAX_AIRQ_AREAS);
What happens if this fails?
Bad things could happen!
How about adding
if (!summary_indicators)
virtio_ccw_use_airq = 0; /* fall back to classic */
?
Since it ain't very likely to happen, we could also just fail
virtio_ccw_init() with -ENOMEM.
That is what I'm currently doing in v3.
Regards,
Halil
return ccw_driver_register(_ccw_driver);
}
device_initcall(virtio_ccw_init);
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 3/8] s390/cio: add basic protected virtualization support
On 25.05.19 11:44, Sebastian Ott wrote:
On Thu, 23 May 2019, Michael Mueller wrote:
static struct ccw_device * io_subchannel_allocate_dev(struct subchannel *sch)
{
struct ccw_device *cdev;
+ struct gen_pool *dma_pool;
cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
- if (cdev) {
- cdev->private = kzalloc(sizeof(struct ccw_device_private),
- GFP_KERNEL | GFP_DMA);
- if (cdev->private)
- return cdev;
- }
+ if (!cdev)
+ goto err_cdev;
+ cdev->private = kzalloc(sizeof(struct ccw_device_private),
+ GFP_KERNEL | GFP_DMA);
+ if (!cdev->private)
+ goto err_priv;
+ cdev->dev.coherent_dma_mask = sch->dev.coherent_dma_mask;
+ cdev->dev.dma_mask = >dev.coherent_dma_mask;
+ dma_pool = cio_gp_dma_create(>dev, 1);
This can return NULL. gen_pool_alloc will panic in this case.
[...]
yep, will handled in next version
+err_dma_area:
+ kfree(io_priv);
one tab gone
Indentation.
+err_priv:
+ put_device(>dev);
+ return ERR_PTR(-ENOMEM);
}
[...]
void ccw_device_update_sense_data(struct ccw_device *cdev)
{
memset(>id, 0, sizeof(cdev->id));
- cdev->id.cu_type = cdev->private->senseid.cu_type;
- cdev->id.cu_model = cdev->private->senseid.cu_model;
- cdev->id.dev_type = cdev->private->senseid.dev_type;
- cdev->id.dev_model = cdev->private->senseid.dev_model;
+ cdev->id.cu_type =
+ cdev->private->dma_area->senseid.cu_type;
+ cdev->id.cu_model =
+ cdev->private->dma_area->senseid.cu_model;
+ cdev->id.dev_type =
+ cdev->private->dma_area->senseid.dev_type;
+ cdev->id.dev_model =
+ cdev->private->dma_area->senseid.dev_model;
These fit into one line.
yep, surprisingly below 80 characters
+/**
+ * Allocate zeroed dma coherent 31 bit addressable memory using
+ * the subchannels dma pool. Maximal size of allocation supported
+ * is PAGE_SIZE.
+ */
drivers/s390/cio/device_ops.c:708: warning: Function parameter or member 'cdev'
not described in 'ccw_device_dma_zalloc'
drivers/s390/cio/device_ops.c:708: warning: Function parameter or member 'size'
not described in 'ccw_device_dma_zalloc'
changing comment open token
Reviewed-by: Sebastian Ott
Thanks!
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 3/8] s390/cio: add basic protected virtualization support
On 27.05.19 12:38, Cornelia Huck wrote:
On Thu, 23 May 2019 18:22:04 +0200
Michael Mueller wrote:
From: Halil Pasic
As virtio-ccw devices are channel devices, we need to use the dma area
for any communication with the hypervisor.
It handles neither QDIO in the common code, nor any device type specific
stuff (like channel programs constructed by the DASD driver).
An interesting side effect is that virtio structures are now going to
get allocated in 31 bit addressable storage.
Signed-off-by: Halil Pasic
[Side note: you really should add your s-o-b if you send someone else's
patches... if Halil ends up committing them, it's fine, though.]
My real problem here is that Halil is writing comments and patches after
I have prepared all my changes. ;) And now this contnues...
Michael
---
arch/s390/include/asm/ccwdev.h | 4 +++
drivers/s390/cio/ccwreq.c| 9 +++---
drivers/s390/cio/device.c| 64 +---
drivers/s390/cio/device_fsm.c| 53 -
drivers/s390/cio/device_id.c | 20 +++--
drivers/s390/cio/device_ops.c| 21 +++--
drivers/s390/cio/device_pgid.c | 22 +++---
drivers/s390/cio/device_status.c | 24 +++
drivers/s390/cio/io_sch.h| 20 +
drivers/s390/virtio/virtio_ccw.c | 10 ---
10 files changed, 164 insertions(+), 83 deletions(-)
(...)
@@ -1593,20 +1622,31 @@ struct ccw_device * __init
ccw_device_create_console(struct ccw_driver *drv)
return ERR_CAST(sch);
io_priv = kzalloc(sizeof(*io_priv), GFP_KERNEL | GFP_DMA);
- if (!io_priv) {
- put_device(>dev);
- return ERR_PTR(-ENOMEM);
- }
+ if (!io_priv)
+ goto err_priv;
+ io_priv->dma_area = dma_alloc_coherent(>dev,
+ sizeof(*io_priv->dma_area),
+ _priv->dma_area_dma, GFP_KERNEL);
Even though we'll only end up here for 3215 or 3270 consoles, this sent
me looking.
This code is invoked via console_init(). A few lines down in
start_kernel(), we have
/*
* This needs to be called before any devices perform DMA
* operations that might use the SWIOTLB bounce buffers. It will
* mark the bounce buffers as decrypted so that their usage will
* not cause "plain-text" data to be decrypted when accessed.
*/
mem_encrypt_init();
So, I'm wondering if creating the console device interacts in any way
with the memory encryption interface?
[Does basic recognition work if you start a protected virt guest with a
3270 console? I realize that the console is unlikely to work, but that
should at least exercise this code path.]
+ if (!io_priv->dma_area)
+ goto err_dma_area;
set_io_private(sch, io_priv);
cdev = io_subchannel_create_ccwdev(sch);
if (IS_ERR(cdev)) {
put_device(>dev);
+ dma_free_coherent(>dev, sizeof(*io_priv->dma_area),
+ io_priv->dma_area, io_priv->dma_area_dma);
kfree(io_priv);
return cdev;
}
cdev->drv = drv;
ccw_device_set_int_class(cdev);
return cdev;
+
+err_dma_area:
+ kfree(io_priv);
+err_priv:
+ put_device(>dev);
+ return ERR_PTR(-ENOMEM);
}
void __init ccw_device_destroy_console(struct ccw_device *cdev)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/8] s390/cio: introduce DMA pools to cio
On 27.05.19 08:57, Cornelia Huck wrote:
On Thu, 23 May 2019 18:22:03 +0200
Michael Mueller wrote:
From: Halil Pasic
To support protected virtualization cio will need to make sure the
memory used for communication with the hypervisor is DMA memory.
Let us introduce one global cio, and some tools for pools seated
"one global pool for cio"?
changed in v3
at individual devices.
Our DMA pools are implemented as a gen_pool backed with DMA pages. The
idea is to avoid each allocation effectively wasting a page, as we
typically allocate much less than PAGE_SIZE.
Signed-off-by: Halil Pasic
---
arch/s390/Kconfig | 1 +
arch/s390/include/asm/cio.h | 11 +
drivers/s390/cio/css.c | 110
3 files changed, 122 insertions(+)
(...)
@@ -1018,6 +1024,109 @@ static struct notifier_block css_power_notifier = {
.notifier_call = css_power_event,
};
+#define POOL_INIT_PAGES 1
+static struct gen_pool *cio_dma_pool;
+/* Currently cio supports only a single css */
This comment looks misplaced.
gone in v3
+#define CIO_DMA_GFP (GFP_KERNEL | __GFP_ZERO)
+
+
+struct device *cio_get_dma_css_dev(void)
+{
+ return _subsystems[0]->device;
+}
+
+struct gen_pool *cio_gp_dma_create(struct device *dma_dev, int nr_pages)
+{
+ struct gen_pool *gp_dma;
+ void *cpu_addr;
+ dma_addr_t dma_addr;
+ int i;
+
+ gp_dma = gen_pool_create(3, -1);
+ if (!gp_dma)
+ return NULL;
+ for (i = 0; i < nr_pages; ++i) {
+ cpu_addr = dma_alloc_coherent(dma_dev, PAGE_SIZE, _addr,
+ CIO_DMA_GFP);
+ if (!cpu_addr)
+ return gp_dma;
So, you may return here with no memory added to the pool at all (or
less than requested), but for the caller that is indistinguishable from
an allocation that went all right. May that be a problem?
Halil,
can you pls. bring some light into the intention of this part of
the code. To me this seems to be odd as well!
Currently cio_gp_dma_create() might succeed with a successful
gen_pool_create() and an initially failing dma_alloc_coherent().
+ gen_pool_add_virt(gp_dma, (unsigned long) cpu_addr,
+ dma_addr, PAGE_SIZE, -1);
+ }
+ return gp_dma;
+}
+
(...)
+static void __init cio_dma_pool_init(void)
+{
+ /* No need to free up the resources: compiled in */
+ cio_dma_pool = cio_gp_dma_create(cio_get_dma_css_dev(), 1);
Does it make sense to continue if you did not get a pool here? I don't
think that should happen unless things were really bad already?
cio_gp_dma_create() will be evaluated and css_bus_init() will fail
in v3 in the NULL case.
+}
+
+void *cio_gp_dma_zalloc(struct gen_pool *gp_dma, struct device *dma_dev,
+ size_t size)
+{
+ dma_addr_t dma_addr;
+ unsigned long addr;
+ size_t chunk_size;
+
+ addr = gen_pool_alloc(gp_dma, size);
+ while (!addr) {
+ chunk_size = round_up(size, PAGE_SIZE);
+ addr = (unsigned long) dma_alloc_coherent(dma_dev,
+chunk_size, _addr, CIO_DMA_GFP);
+ if (!addr)
+ return NULL;
+ gen_pool_add_virt(gp_dma, addr, dma_addr, chunk_size, -1);
+ addr = gen_pool_alloc(gp_dma, size);
+ }
+ return (void *) addr;
+}
+
+void cio_gp_dma_free(struct gen_pool *gp_dma, void *cpu_addr, size_t size)
+{
+ if (!cpu_addr)
+ return;
+ memset(cpu_addr, 0, size);
+ gen_pool_free(gp_dma, (unsigned long) cpu_addr, size);
+}
+
+/**
+ * Allocate dma memory from the css global pool. Intended for memory not
+ * specific to any single device within the css. The allocated memory
+ * is not guaranteed to be 31-bit addressable.
+ *
+ * Caution: Not suitable for early stuff like console.
+ *
+ */
+void *cio_dma_zalloc(size_t size)
+{
+ return cio_gp_dma_zalloc(cio_dma_pool, cio_get_dma_css_dev(), size);
Ok, that looks like the failure I mentioned above should be
accommodated by the code. Still, I think it's a bit odd.
This code will be reached in v3 only when cio_dma_pool is *not* NULL.
+}
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/8] s390/cio: introduce DMA pools to cio
On 25.05.19 11:22, Sebastian Ott wrote:
On Thu, 23 May 2019, Michael Mueller wrote:
+static void __init cio_dma_pool_init(void)
+{
+ /* No need to free up the resources: compiled in */
+ cio_dma_pool = cio_gp_dma_create(cio_get_dma_css_dev(), 1);
This can return NULL.
css_bus_init() will fail with -ENOMEM in v3
+/**
+ * Allocate dma memory from the css global pool. Intended for memory not
+ * specific to any single device within the css. The allocated memory
+ * is not guaranteed to be 31-bit addressable.
+ *
+ * Caution: Not suitable for early stuff like console.
+ *
+ */
drivers/s390/cio/css.c:1121: warning: Function parameter or member 'size' not
described in 'cio_dma_zalloc'
will complete param description in v3
Reviewed-by: Sebastian Ott
Thanks!
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 6/8] virtio/s390: add indirection to indicators access
From: Halil Pasic
This will come in handy soon when we pull out the indicators from
virtio_ccw_device to a memory area that is shared with the hypervisor
(in particular for protected virtualization guests).
Signed-off-by: Halil Pasic
Reviewed-by: Pierre Morel
---
drivers/s390/virtio/virtio_ccw.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 1da7430f94c8..e96a8cc56ec2 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -68,6 +68,16 @@ struct virtio_ccw_device {
void *airq_info;
};
+static inline unsigned long *indicators(struct virtio_ccw_device *vcdev)
+{
+ return >indicators;
+}
+
+static inline unsigned long *indicators2(struct virtio_ccw_device *vcdev)
+{
+ return >indicators2;
+}
+
struct vq_info_block_legacy {
__u64 queue;
__u32 align;
@@ -338,17 +348,17 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
ccw->cda = (__u32)(unsigned long) thinint_area;
} else {
/* payload is the address of the indicators */
- indicatorp = kmalloc(sizeof(>indicators),
+ indicatorp = kmalloc(sizeof(indicators(vcdev)),
GFP_DMA | GFP_KERNEL);
if (!indicatorp)
return;
*indicatorp = 0;
ccw->cmd_code = CCW_CMD_SET_IND;
- ccw->count = sizeof(>indicators);
+ ccw->count = sizeof(indicators(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
}
/* Deregister indicators from host. */
- vcdev->indicators = 0;
+ *indicators(vcdev) = 0;
ccw->flags = 0;
ret = ccw_io_helper(vcdev, ccw,
vcdev->is_thinint ?
@@ -657,10 +667,10 @@ static int virtio_ccw_find_vqs(struct virtio_device
*vdev, unsigned nvqs,
* We need a data area under 2G to communicate. Our payload is
* the address of the indicators.
*/
- indicatorp = kmalloc(sizeof(>indicators), GFP_DMA | GFP_KERNEL);
+ indicatorp = kmalloc(sizeof(indicators(vcdev)), GFP_DMA | GFP_KERNEL);
if (!indicatorp)
goto out;
- *indicatorp = (unsigned long) >indicators;
+ *indicatorp = (unsigned long) indicators(vcdev);
if (vcdev->is_thinint) {
ret = virtio_ccw_register_adapter_ind(vcdev, vqs, nvqs, ccw);
if (ret)
@@ -669,21 +679,21 @@ static int virtio_ccw_find_vqs(struct virtio_device
*vdev, unsigned nvqs,
}
if (!vcdev->is_thinint) {
/* Register queue indicators with host. */
- vcdev->indicators = 0;
+ *indicators(vcdev) = 0;
ccw->cmd_code = CCW_CMD_SET_IND;
ccw->flags = 0;
- ccw->count = sizeof(>indicators);
+ ccw->count = sizeof(indicators(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_SET_IND);
if (ret)
goto out;
}
/* Register indicators2 with host for config changes */
- *indicatorp = (unsigned long) >indicators2;
- vcdev->indicators2 = 0;
+ *indicatorp = (unsigned long) indicators2(vcdev);
+ *indicators2(vcdev) = 0;
ccw->cmd_code = CCW_CMD_SET_CONF_IND;
ccw->flags = 0;
- ccw->count = sizeof(>indicators2);
+ ccw->count = sizeof(indicators2(vcdev));
ccw->cda = (__u32)(unsigned long) indicatorp;
ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_SET_CONF_IND);
if (ret)
@@ -1093,17 +1103,17 @@ static void virtio_ccw_int_handler(struct ccw_device
*cdev,
vcdev->err = -EIO;
}
virtio_ccw_check_activity(vcdev, activity);
- for_each_set_bit(i, >indicators,
-sizeof(vcdev->indicators) * BITS_PER_BYTE) {
+ for_each_set_bit(i, indicators(vcdev),
+sizeof(*indicators(vcdev)) * BITS_PER_BYTE) {
/* The bit clear must happen before the vring kick. */
- clear_bit(i, >indicators);
+ clear_bit(i, indicators(vcdev));
barrier();
vq = virtio_ccw_vq_by_ind(vcdev, i);
vring_interrupt(0, vq);
}
- if (test_bit(0, >indicators2)) {
+ if (test_bit(0, indicators2(vcdev))) {
virtio_config_changed(>vdev);
- clear_bit(0, >indicators2);
+ clear_bit(0, indicators2(vcdev));
}
}
--
2.13.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 8/8] virtio/s390: make airq summary indicators DMA
From: Halil Pasic
Hypervisor needs to interact with the summary indicators, so these
need to be DMA memory as well (at least for protected virtualization
guests).
Signed-off-by: Halil Pasic
---
drivers/s390/virtio/virtio_ccw.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 03c9f7001fb1..f666ed397dc0 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -140,11 +140,17 @@ static int virtio_ccw_use_airq = 1;
struct airq_info {
rwlock_t lock;
- u8 summary_indicator;
+ u8 summary_indicator_idx;
struct airq_struct airq;
struct airq_iv *aiv;
};
static struct airq_info *airq_areas[MAX_AIRQ_AREAS];
+static u8 *summary_indicators;
+
+static inline u8 *get_summary_indicator(struct airq_info *info)
+{
+ return summary_indicators + info->summary_indicator_idx;
+}
#define CCW_CMD_SET_VQ 0x13
#define CCW_CMD_VDEV_RESET 0x33
@@ -225,7 +231,7 @@ static void virtio_airq_handler(struct airq_struct *airq,
bool floating)
break;
vring_interrupt(0, (void *)airq_iv_get_ptr(info->aiv, ai));
}
- info->summary_indicator = 0;
+ *(get_summary_indicator(info)) = 0;
smp_wmb();
/* Walk through indicators field, summary indicator not active. */
for (ai = 0;;) {
@@ -237,7 +243,7 @@ static void virtio_airq_handler(struct airq_struct *airq,
bool floating)
read_unlock(>lock);
}
-static struct airq_info *new_airq_info(void)
+static struct airq_info *new_airq_info(int index)
{
struct airq_info *info;
int rc;
@@ -253,7 +259,8 @@ static struct airq_info *new_airq_info(void)
return NULL;
}
info->airq.handler = virtio_airq_handler;
- info->airq.lsi_ptr = >summary_indicator;
+ info->summary_indicator_idx = index;
+ info->airq.lsi_ptr = get_summary_indicator(info);
info->airq.lsi_mask = 0xff;
info->airq.isc = VIRTIO_AIRQ_ISC;
rc = register_adapter_interrupt(>airq);
@@ -275,7 +282,7 @@ static unsigned long get_airq_indicator(struct virtqueue
*vqs[], int nvqs,
for (i = 0; i < MAX_AIRQ_AREAS && !indicator_addr; i++) {
if (!airq_areas[i])
- airq_areas[i] = new_airq_info();
+ airq_areas[i] = new_airq_info(i);
info = airq_areas[i];
if (!info)
return 0;
@@ -360,7 +367,7 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
if (!thinint_area)
return;
thinint_area->summary_indicator =
- (unsigned long) _info->summary_indicator;
+ (unsigned long) get_summary_indicator(airq_info);
thinint_area->isc = VIRTIO_AIRQ_ISC;
ccw->cmd_code = CCW_CMD_SET_IND_ADAPTER;
ccw->count = sizeof(*thinint_area);
@@ -625,7 +632,7 @@ static int virtio_ccw_register_adapter_ind(struct
virtio_ccw_device *vcdev,
}
info = vcdev->airq_info;
thinint_area->summary_indicator =
- (unsigned long) >summary_indicator;
+ (unsigned long) get_summary_indicator(info);
thinint_area->isc = VIRTIO_AIRQ_ISC;
ccw->cmd_code = CCW_CMD_SET_IND_ADAPTER;
ccw->flags = CCW_FLAG_SLI;
@@ -1501,6 +1508,7 @@ static int __init virtio_ccw_init(void)
{
/* parse no_auto string before we do anything further */
no_auto_parse();
+ summary_indicators = cio_dma_zalloc(MAX_AIRQ_AREAS);
return ccw_driver_register(_ccw_driver);
}
device_initcall(virtio_ccw_init);
--
2.13.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 7/8] virtio/s390: use DMA memory for ccw I/O and classic notifiers
From: Halil Pasic
Before virtio-ccw could get away with not using DMA API for the pieces of
memory it does ccw I/O with. With protected virtualization this has to
change, since the hypervisor needs to read and sometimes also write these
pieces of memory.
The hypervisor is supposed to poke the classic notifiers, if these are
used, out of band with regards to ccw I/O. So these need to be allocated
as DMA memory (which is shared memory for protected virtualization
guests).
Let us factor out everything from struct virtio_ccw_device that needs to
be DMA memory in a satellite that is allocated as such.
Note: The control blocks of I/O instructions do not need to be shared.
These are marshalled by the ultravisor.
Signed-off-by: Halil Pasic
Reviewed-by: Pierre Morel
---
drivers/s390/virtio/virtio_ccw.c | 177 +--
1 file changed, 96 insertions(+), 81 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index e96a8cc56ec2..03c9f7001fb1 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -46,9 +46,15 @@ struct vq_config_block {
#define VIRTIO_CCW_CONFIG_SIZE 0x100
/* same as PCI config space size, should be enough for all drivers */
+struct vcdev_dma_area {
+ unsigned long indicators;
+ unsigned long indicators2;
+ struct vq_config_block config_block;
+ __u8 status;
+};
+
struct virtio_ccw_device {
struct virtio_device vdev;
- __u8 *status;
__u8 config[VIRTIO_CCW_CONFIG_SIZE];
struct ccw_device *cdev;
__u32 curr_io;
@@ -58,24 +64,22 @@ struct virtio_ccw_device {
spinlock_t lock;
struct mutex io_lock; /* Serializes I/O requests */
struct list_head virtqueues;
- unsigned long indicators;
- unsigned long indicators2;
- struct vq_config_block *config_block;
bool is_thinint;
bool going_away;
bool device_lost;
unsigned int config_ready;
void *airq_info;
+ struct vcdev_dma_area *dma_area;
};
static inline unsigned long *indicators(struct virtio_ccw_device *vcdev)
{
- return >indicators;
+ return >dma_area->indicators;
}
static inline unsigned long *indicators2(struct virtio_ccw_device *vcdev)
{
- return >indicators2;
+ return >dma_area->indicators2;
}
struct vq_info_block_legacy {
@@ -176,6 +180,22 @@ static struct virtio_ccw_device *to_vc_device(struct
virtio_device *vdev)
return container_of(vdev, struct virtio_ccw_device, vdev);
}
+static inline void *__vc_dma_alloc(struct virtio_device *vdev, size_t size)
+{
+ return ccw_device_dma_zalloc(to_vc_device(vdev)->cdev, size);
+}
+
+static inline void __vc_dma_free(struct virtio_device *vdev, size_t size,
+void *cpu_addr)
+{
+ return ccw_device_dma_free(to_vc_device(vdev)->cdev, cpu_addr, size);
+}
+
+#define vc_dma_alloc_struct(vdev, ptr) \
+ ({ptr = __vc_dma_alloc(vdev, sizeof(*(ptr))); })
+#define vc_dma_free_struct(vdev, ptr) \
+ __vc_dma_free(vdev, sizeof(*(ptr)), (ptr))
+
static void drop_airq_indicator(struct virtqueue *vq, struct airq_info *info)
{
unsigned long i, flags;
@@ -336,8 +356,7 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
struct airq_info *airq_info = vcdev->airq_info;
if (vcdev->is_thinint) {
- thinint_area = kzalloc(sizeof(*thinint_area),
- GFP_DMA | GFP_KERNEL);
+ vc_dma_alloc_struct(>vdev, thinint_area);
if (!thinint_area)
return;
thinint_area->summary_indicator =
@@ -348,8 +367,8 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
ccw->cda = (__u32)(unsigned long) thinint_area;
} else {
/* payload is the address of the indicators */
- indicatorp = kmalloc(sizeof(indicators(vcdev)),
-GFP_DMA | GFP_KERNEL);
+ indicatorp = __vc_dma_alloc(>vdev,
+ sizeof(indicators(vcdev)));
if (!indicatorp)
return;
*indicatorp = 0;
@@ -369,8 +388,9 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
"Failed to deregister indicators (%d)\n", ret);
else if (vcdev->is_thinint)
virtio_ccw_drop_indicators(vcdev);
- kfree(indicatorp);
- kfree(thinint_area);
+ __vc_dma_free(>vdev, sizeof(indicators(vcdev)),
+ indicatorp);
+ vc_dma_free_struct(>vdev, thinint_area);
}
static inline long __do_kvm_notify(struct subchannel_id schid,
@@ -417,15 +437,15 @@ static int virtio_ccw_read_vq_conf(struct
virtio_ccw_device *vcdev,
{
int ret;
- vcdev->config_block->index = index;
+
[PATCH v2 5/8] virtio/s390: use cacheline aligned airq bit vectors
From: Halil Pasic
The flag AIRQ_IV_CACHELINE was recently added to airq_iv_create(). Let
us use it! We actually wanted the vector to span a cacheline all along.
Signed-off-by: Halil Pasic
---
drivers/s390/virtio/virtio_ccw.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index f995798bb025..1da7430f94c8 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -216,7 +216,8 @@ static struct airq_info *new_airq_info(void)
if (!info)
return NULL;
rwlock_init(>lock);
- info->aiv = airq_iv_create(VIRTIO_IV_BITS, AIRQ_IV_ALLOC | AIRQ_IV_PTR);
+ info->aiv = airq_iv_create(VIRTIO_IV_BITS, AIRQ_IV_ALLOC | AIRQ_IV_PTR
+ | AIRQ_IV_CACHELINE);
if (!info->aiv) {
kfree(info);
return NULL;
--
2.13.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 4/8] s390/airq: use DMA memory for adapter interrupts
From: Halil Pasic
Protected virtualization guests have to use shared pages for airq
notifier bit vectors, because hypervisor needs to write these bits.
Let us make sure we allocate DMA memory for the notifier bit vectors by
replacing the kmem_cache with a dma_cache and kalloc() with
cio_dma_zalloc().
Signed-off-by: Halil Pasic
---
arch/s390/include/asm/airq.h | 2 ++
drivers/s390/cio/airq.c | 32
drivers/s390/cio/cio.h | 2 ++
drivers/s390/cio/css.c | 1 +
4 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h
index c10d2ee2dfda..01936fdfaddb 100644
--- a/arch/s390/include/asm/airq.h
+++ b/arch/s390/include/asm/airq.h
@@ -11,6 +11,7 @@
#define _ASM_S390_AIRQ_H
#include
+#include
struct airq_struct {
struct hlist_node list; /* Handler queueing. */
@@ -29,6 +30,7 @@ void unregister_adapter_interrupt(struct airq_struct *airq);
/* Adapter interrupt bit vector */
struct airq_iv {
unsigned long *vector; /* Adapter interrupt bit vector */
+ dma_addr_t vector_dma; /* Adapter interrupt bit vector dma */
unsigned long *avail; /* Allocation bit mask for the bit vector */
unsigned long *bitlock; /* Lock bit mask for the bit vector */
unsigned long *ptr; /* Pointer associated with each bit */
diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c
index 4534afc63591..89d26e43004d 100644
--- a/drivers/s390/cio/airq.c
+++ b/drivers/s390/cio/airq.c
@@ -16,9 +16,11 @@
#include
#include
#include
+#include
#include
#include
+#include
#include "cio.h"
#include "cio_debug.h"
@@ -27,7 +29,7 @@
static DEFINE_SPINLOCK(airq_lists_lock);
static struct hlist_head airq_lists[MAX_ISC+1];
-static struct kmem_cache *airq_iv_cache;
+static struct dma_pool *airq_iv_cache;
/**
* register_adapter_interrupt() - register adapter interrupt handler
@@ -115,6 +117,11 @@ void __init init_airq_interrupts(void)
setup_irq(THIN_INTERRUPT, _interrupt);
}
+static inline unsigned long iv_size(unsigned long bits)
+{
+ return BITS_TO_LONGS(bits) * sizeof(unsigned long);
+}
+
/**
* airq_iv_create - create an interrupt vector
* @bits: number of bits in the interrupt vector
@@ -132,17 +139,18 @@ struct airq_iv *airq_iv_create(unsigned long bits,
unsigned long flags)
goto out;
iv->bits = bits;
iv->flags = flags;
- size = BITS_TO_LONGS(bits) * sizeof(unsigned long);
+ size = iv_size(bits);
if (flags & AIRQ_IV_CACHELINE) {
if ((cache_line_size() * BITS_PER_BYTE) < bits)
goto out_free;
- iv->vector = kmem_cache_zalloc(airq_iv_cache, GFP_KERNEL);
+ iv->vector = dma_pool_zalloc(airq_iv_cache, GFP_KERNEL,
+>vector_dma);
if (!iv->vector)
goto out_free;
} else {
- iv->vector = kzalloc(size, GFP_KERNEL);
+ iv->vector = cio_dma_zalloc(size);
if (!iv->vector)
goto out_free;
}
@@ -179,9 +187,9 @@ struct airq_iv *airq_iv_create(unsigned long bits, unsigned
long flags)
kfree(iv->bitlock);
kfree(iv->avail);
if (iv->flags & AIRQ_IV_CACHELINE)
- kmem_cache_free(airq_iv_cache, iv->vector);
+ dma_pool_free(airq_iv_cache, iv->vector, iv->vector_dma);
else
- kfree(iv->vector);
+ cio_dma_free(iv->vector, size);
kfree(iv);
out:
return NULL;
@@ -198,9 +206,9 @@ void airq_iv_release(struct airq_iv *iv)
kfree(iv->ptr);
kfree(iv->bitlock);
if (iv->flags & AIRQ_IV_CACHELINE)
- kmem_cache_free(airq_iv_cache, iv->vector);
+ dma_pool_free(airq_iv_cache, iv->vector, iv->vector_dma);
else
- kfree(iv->vector);
+ cio_dma_free(iv->vector, iv_size(iv->bits));
kfree(iv->avail);
kfree(iv);
}
@@ -295,12 +303,12 @@ unsigned long airq_iv_scan(struct airq_iv *iv, unsigned
long start,
}
EXPORT_SYMBOL(airq_iv_scan);
-static int __init airq_init(void)
+int __init airq_init(void)
{
- airq_iv_cache = kmem_cache_create("airq_iv_cache", cache_line_size(),
- cache_line_size(), 0, NULL);
+ airq_iv_cache = dma_pool_create("airq_iv_cache", cio_get_dma_css_dev(),
+ cache_line_size(),
+ cache_line_size(), PAGE_SIZE);
if (!airq_iv_cache)
return -ENOMEM;
return 0;
}
-subsys_initcall(airq_init);
diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h
index 06a91743335a..4d6c7d16416e 100644
--- a/drivers/s390/cio/cio.h
+++ b/drivers/s390/cio/cio.h
@@ -135,6 +135,8 @@ extern int cio_commit_config(struct
[PATCH v2 3/8] s390/cio: add basic protected virtualization support
From: Halil Pasic
As virtio-ccw devices are channel devices, we need to use the dma area
for any communication with the hypervisor.
It handles neither QDIO in the common code, nor any device type specific
stuff (like channel programs constructed by the DASD driver).
An interesting side effect is that virtio structures are now going to
get allocated in 31 bit addressable storage.
Signed-off-by: Halil Pasic
---
arch/s390/include/asm/ccwdev.h | 4 +++
drivers/s390/cio/ccwreq.c| 9 +++---
drivers/s390/cio/device.c| 64 +---
drivers/s390/cio/device_fsm.c| 53 -
drivers/s390/cio/device_id.c | 20 +++--
drivers/s390/cio/device_ops.c| 21 +++--
drivers/s390/cio/device_pgid.c | 22 +++---
drivers/s390/cio/device_status.c | 24 +++
drivers/s390/cio/io_sch.h| 20 +
drivers/s390/virtio/virtio_ccw.c | 10 ---
10 files changed, 164 insertions(+), 83 deletions(-)
diff --git a/arch/s390/include/asm/ccwdev.h b/arch/s390/include/asm/ccwdev.h
index a29dd430fb40..865ce1cb86d5 100644
--- a/arch/s390/include/asm/ccwdev.h
+++ b/arch/s390/include/asm/ccwdev.h
@@ -226,6 +226,10 @@ extern int ccw_device_enable_console(struct ccw_device *);
extern void ccw_device_wait_idle(struct ccw_device *);
extern int ccw_device_force_console(struct ccw_device *);
+extern void *ccw_device_dma_zalloc(struct ccw_device *cdev, size_t size);
+extern void ccw_device_dma_free(struct ccw_device *cdev,
+ void *cpu_addr, size_t size);
+
int ccw_device_siosl(struct ccw_device *);
extern void ccw_device_get_schid(struct ccw_device *, struct subchannel_id *);
diff --git a/drivers/s390/cio/ccwreq.c b/drivers/s390/cio/ccwreq.c
index 603268a33ea1..73582a0a2622 100644
--- a/drivers/s390/cio/ccwreq.c
+++ b/drivers/s390/cio/ccwreq.c
@@ -63,7 +63,7 @@ static void ccwreq_stop(struct ccw_device *cdev, int rc)
return;
req->done = 1;
ccw_device_set_timeout(cdev, 0);
- memset(>private->irb, 0, sizeof(struct irb));
+ memset(>private->dma_area->irb, 0, sizeof(struct irb));
if (rc && rc != -ENODEV && req->drc)
rc = req->drc;
req->callback(cdev, req->data, rc);
@@ -86,7 +86,7 @@ static void ccwreq_do(struct ccw_device *cdev)
continue;
}
/* Perform start function. */
- memset(>private->irb, 0, sizeof(struct irb));
+ memset(>private->dma_area->irb, 0, sizeof(struct irb));
rc = cio_start(sch, cp, (u8) req->mask);
if (rc == 0) {
/* I/O started successfully. */
@@ -169,7 +169,7 @@ int ccw_request_cancel(struct ccw_device *cdev)
*/
static enum io_status ccwreq_status(struct ccw_device *cdev, struct irb *lcirb)
{
- struct irb *irb = >private->irb;
+ struct irb *irb = >private->dma_area->irb;
struct cmd_scsw *scsw = >scsw.cmd;
enum uc_todo todo;
@@ -187,7 +187,8 @@ static enum io_status ccwreq_status(struct ccw_device
*cdev, struct irb *lcirb)
CIO_TRACE_EVENT(2, "sensedata");
CIO_HEX_EVENT(2, >private->dev_id,
sizeof(struct ccw_dev_id));
- CIO_HEX_EVENT(2, >private->irb.ecw, SENSE_MAX_COUNT);
+ CIO_HEX_EVENT(2, >private->dma_area->irb.ecw,
+ SENSE_MAX_COUNT);
/* Check for command reject. */
if (irb->ecw[0] & SNS0_CMD_REJECT)
return IO_REJECTED;
diff --git a/drivers/s390/cio/device.c b/drivers/s390/cio/device.c
index 1540229a37bb..a9ee03c20d7e 100644
--- a/drivers/s390/cio/device.c
+++ b/drivers/s390/cio/device.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
@@ -687,6 +688,9 @@ ccw_device_release(struct device *dev)
struct ccw_device *cdev;
cdev = to_ccwdev(dev);
+ cio_gp_dma_free(cdev->private->dma_pool, cdev->private->dma_area,
+ sizeof(*cdev->private->dma_area));
+ cio_gp_dma_destroy(cdev->private->dma_pool, >dev);
/* Release reference of parent subchannel. */
put_device(cdev->dev.parent);
kfree(cdev->private);
@@ -696,15 +700,30 @@ ccw_device_release(struct device *dev)
static struct ccw_device * io_subchannel_allocate_dev(struct subchannel *sch)
{
struct ccw_device *cdev;
+ struct gen_pool *dma_pool;
cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
- if (cdev) {
- cdev->private = kzalloc(sizeof(struct ccw_device_private),
- GFP_KERNEL | GFP_DMA);
- if (cdev->private)
- return cdev;
- }
+ if (!cdev)
+ goto err_cdev;
+ cdev->private = kzalloc(sizeof(struct ccw_device_private),
+
[PATCH v2 0/8] s390: virtio: support protected virtualization
Enhanced virtualization protection technology may require the use of bounce buffers for I/O. While support for this was built into the virtio core, virtio-ccw wasn't changed accordingly. Some background on technology (not part of this series) and the terminology used. * Protected Virtualization (PV): Protected Virtualization guarantees, that non-shared memory of a guest that operates in PV mode private to that guest. I.e. any attempts by the hypervisor or other guests to access it will result in an exception. If supported by the environment (machine, KVM, guest VM) a guest can decide to change into PV mode by doing the appropriate ultravisor calls. * Ultravisor: A hardware/firmware entity that manages PV guests, and polices access to their memory. A PV guest prospect needs to interact with the ultravisor, to enter PV mode, and potentially to share pages (for I/O which should be encrypted by the guest). A guest interacts with the ultravisor via so called ultravisor calls. A hypervisor needs to interact with the ultravisor to facilitate interpretation, emulation and swapping. A hypervisor interacts with the ultravisor via ultravisor calls and via the SIE state description. Generally the ultravisor sanitizes hypervisor inputs so that the guest can not be corrupted (except for denial of service. What needs to be done = Thus what needs to be done to bring virtio-ccw up to speed with respect to protected virtualization is: * use some 'new' common virtio stuff * make sure that virtio-ccw specific stuff uses shared memory when talking to the hypervisor (except control/communication blocks like ORB, these are handled by the ultravisor) * make sure the DMA API does what is necessary to talk through shared memory if we are a protected virtualization guest. * make sure the common IO layer plays along as well (airqs, sense). Important notes * This patch set is based on Martins features branch (git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git branch 'features'). * Documentation is still very sketchy. I'm committed to improving this, but I'm currently hampered by some dependencies currently. * The existing naming in the common infrastructure (kernel internal interfaces) is pretty much based on the AMD SEV terminology. Thus the names aren't always perfect. There might be merit to changing these names to more abstract ones. I did not put much thought into that at the current stage. * Testing: Please use iommu_platform=on for any virtio devices you are going to test this code with (so virtio actually uses the DMA API). Change log == v1 --> v2: * patch "virtio/s390: use vring_create_virtqueue" went already upstream * patch "virtio/s390: DMA support for virtio-ccw" went already upstream * patch "virtio/s390: enable packed ring" went already upstream * Made dev.dma_mask point to dev.coherent_dma_mask for css, subchannel and ccw devices. * While rebasing 's390/airq: use DMA memory for adapter interrupts' the newly introduced kmem_cache was replaced with an equivalent dma_pool; the kalloc() allocations are now replaced with cio_dma_zalloc() allocations to avoid wasting almost a full page. * Made virtio-ccw use the new AIRQ_IV_CACHELINE flag. * fixed all remaining checkpatch issues RFC --> v1: * Fixed bugs found by Connie (may_reduce and handling reduced, warning, split move -- thanks Connie!). * Fixed console bug found by Sebastian (thanks Sebastian!). * Removed the completely useless duplicate of dma-mapping.h spotted by Christoph (thanks Christoph!). * Don't use the global DMA pool for subchannel and ccw device owned memory as requested by Sebastian. Consequences: * Both subchannel and ccw devices have their dma masks now (both specifying 31 bit addressable) * We require at least 2 DMA pages per ccw device now, most of this memory is wasted though. * DMA memory allocated by virtio is also 31 bit addressable now as virtio uses the parent (which is the ccw device). * Enabled packed ring. * Rebased onto Martins feature branch; using the actual uv (ultravisor) interface instead of TODO comments. * Added some explanations to the cover letter (Connie, David). * Squashed a couple of patches together and fixed some text stuff. Looking forward to your review, or any other type of input. Halil Pasic (8): s390/mm: force swiotlb for protected virtualization s390/cio: introduce DMA pools to cio s390/cio: add basic protected virtualization support s390/airq: use DMA memory for adapter interrupts virtio/s390: use cacheline aligned airq bit vectors virtio/s390: add indirection to indicators access virtio/s390: use DMA memory for ccw I/O and classic notifiers virtio/s390: make airq summary indicators DMA arch/s390/Kconfig | 5 + arch/s390/include/asm/airq.h| 2 + arch/s390/include/asm/ccwdev.h | 4 +
[PATCH v2 1/8] s390/mm: force swiotlb for protected virtualization
From: Halil Pasic
On s390, protected virtualization guests have to use bounced I/O
buffers. That requires some plumbing.
Let us make sure, any device that uses DMA API with direct ops correctly
is spared from the problems, that a hypervisor attempting I/O to a
non-shared page would bring.
Signed-off-by: Halil Pasic
Reviewed-by: Claudio Imbrenda
---
arch/s390/Kconfig | 4
arch/s390/include/asm/mem_encrypt.h | 18 ++
arch/s390/mm/init.c | 47 +
3 files changed, 69 insertions(+)
create mode 100644 arch/s390/include/asm/mem_encrypt.h
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 109243fdb6ec..88d8355b7bf7 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -1,4 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+config ARCH_HAS_MEM_ENCRYPT
+def_bool y
+
config MMU
def_bool y
@@ -187,6 +190,7 @@ config S390
select VIRT_CPU_ACCOUNTING
select ARCH_HAS_SCALED_CPUTIME
select HAVE_NMI
+ select SWIOTLB
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/s390/include/asm/mem_encrypt.h
b/arch/s390/include/asm/mem_encrypt.h
new file mode 100644
index ..0898c09a888c
--- /dev/null
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef S390_MEM_ENCRYPT_H__
+#define S390_MEM_ENCRYPT_H__
+
+#ifndef __ASSEMBLY__
+
+#define sme_me_mask0ULL
+
+static inline bool sme_active(void) { return false; }
+extern bool sev_active(void);
+
+int set_memory_encrypted(unsigned long addr, int numpages);
+int set_memory_decrypted(unsigned long addr, int numpages);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* S390_MEM_ENCRYPT_H__ */
+
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 14d1eae9fe43..f0bee6af3960 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -29,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -42,6 +44,8 @@
#include
#include
#include
+#include
+#include
pgd_t swapper_pg_dir[PTRS_PER_PGD] __section(.bss..swapper_pg_dir);
@@ -128,6 +132,47 @@ void mark_rodata_ro(void)
pr_info("Write protected read-only-after-init data: %luk\n", size >>
10);
}
+int set_memory_encrypted(unsigned long addr, int numpages)
+{
+ int i;
+
+ /* make specified pages unshared, (swiotlb, dma_free) */
+ for (i = 0; i < numpages; ++i) {
+ uv_remove_shared(addr);
+ addr += PAGE_SIZE;
+ }
+ return 0;
+}
+
+int set_memory_decrypted(unsigned long addr, int numpages)
+{
+ int i;
+ /* make specified pages shared (swiotlb, dma_alloca) */
+ for (i = 0; i < numpages; ++i) {
+ uv_set_shared(addr);
+ addr += PAGE_SIZE;
+ }
+ return 0;
+}
+
+/* are we a protected virtualization guest? */
+bool sev_active(void)
+{
+ return is_prot_virt_guest();
+}
+
+/* protected virtualization */
+static void pv_init(void)
+{
+ if (!is_prot_virt_guest())
+ return;
+
+ /* make sure bounce buffers are shared */
+ swiotlb_init(1);
+ swiotlb_update_mem_attributes();
+ swiotlb_force = SWIOTLB_FORCE;
+}
+
void __init mem_init(void)
{
cpumask_set_cpu(0, _mm.context.cpu_attach_mask);
@@ -136,6 +181,8 @@ void __init mem_init(void)
set_max_mapnr(max_low_pfn);
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
+ pv_init();
+
/* Setup guest page hinting */
cmma_init();
--
2.13.4
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 2/8] s390/cio: introduce DMA pools to cio
From: Halil Pasic
To support protected virtualization cio will need to make sure the
memory used for communication with the hypervisor is DMA memory.
Let us introduce one global cio, and some tools for pools seated
at individual devices.
Our DMA pools are implemented as a gen_pool backed with DMA pages. The
idea is to avoid each allocation effectively wasting a page, as we
typically allocate much less than PAGE_SIZE.
Signed-off-by: Halil Pasic
---
arch/s390/Kconfig | 1 +
arch/s390/include/asm/cio.h | 11 +
drivers/s390/cio/css.c | 110
3 files changed, 122 insertions(+)
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 88d8355b7bf7..2a245b56db8b 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -191,6 +191,7 @@ config S390
select ARCH_HAS_SCALED_CPUTIME
select HAVE_NMI
select SWIOTLB
+ select GENERIC_ALLOCATOR
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/s390/include/asm/cio.h b/arch/s390/include/asm/cio.h
index 1727180e8ca1..43c007d2775a 100644
--- a/arch/s390/include/asm/cio.h
+++ b/arch/s390/include/asm/cio.h
@@ -328,6 +328,17 @@ static inline u8 pathmask_to_pos(u8 mask)
void channel_subsystem_reinit(void);
extern void css_schedule_reprobe(void);
+extern void *cio_dma_zalloc(size_t size);
+extern void cio_dma_free(void *cpu_addr, size_t size);
+extern struct device *cio_get_dma_css_dev(void);
+
+struct gen_pool;
+void *cio_gp_dma_zalloc(struct gen_pool *gp_dma, struct device *dma_dev,
+ size_t size);
+void cio_gp_dma_free(struct gen_pool *gp_dma, void *cpu_addr, size_t size);
+void cio_gp_dma_destroy(struct gen_pool *gp_dma, struct device *dma_dev);
+struct gen_pool *cio_gp_dma_create(struct device *dma_dev, int nr_pages);
+
/* Function from drivers/s390/cio/chsc.c */
int chsc_sstpc(void *page, unsigned int op, u16 ctrl, u64 *clock_delta);
int chsc_sstpi(void *page, void *result, size_t size);
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index aea502922646..789f6ecdbbcc 100644
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -20,6 +20,8 @@
#include
#include
#include
+#include
+#include
#include
#include
@@ -224,6 +226,8 @@ struct subchannel *css_alloc_subchannel(struct
subchannel_id schid,
INIT_WORK(>todo_work, css_sch_todo);
sch->dev.release = _subchannel_release;
device_initialize(>dev);
+ sch->dev.coherent_dma_mask = DMA_BIT_MASK(31);
+ sch->dev.dma_mask = >dev.coherent_dma_mask;
return sch;
err:
@@ -899,6 +903,8 @@ static int __init setup_css(int nr)
dev_set_name(>device, "css%x", nr);
css->device.groups = cssdev_attr_groups;
css->device.release = channel_subsystem_release;
+ css->device.coherent_dma_mask = DMA_BIT_MASK(64);
+ css->device.dma_mask = >device.coherent_dma_mask;
mutex_init(>mutex);
css->cssid = chsc_get_cssid(nr);
@@ -1018,6 +1024,109 @@ static struct notifier_block css_power_notifier = {
.notifier_call = css_power_event,
};
+#define POOL_INIT_PAGES 1
+static struct gen_pool *cio_dma_pool;
+/* Currently cio supports only a single css */
+#define CIO_DMA_GFP (GFP_KERNEL | __GFP_ZERO)
+
+
+struct device *cio_get_dma_css_dev(void)
+{
+ return _subsystems[0]->device;
+}
+
+struct gen_pool *cio_gp_dma_create(struct device *dma_dev, int nr_pages)
+{
+ struct gen_pool *gp_dma;
+ void *cpu_addr;
+ dma_addr_t dma_addr;
+ int i;
+
+ gp_dma = gen_pool_create(3, -1);
+ if (!gp_dma)
+ return NULL;
+ for (i = 0; i < nr_pages; ++i) {
+ cpu_addr = dma_alloc_coherent(dma_dev, PAGE_SIZE, _addr,
+ CIO_DMA_GFP);
+ if (!cpu_addr)
+ return gp_dma;
+ gen_pool_add_virt(gp_dma, (unsigned long) cpu_addr,
+ dma_addr, PAGE_SIZE, -1);
+ }
+ return gp_dma;
+}
+
+static void __gp_dma_free_dma(struct gen_pool *pool,
+ struct gen_pool_chunk *chunk, void *data)
+{
+ size_t chunk_size = chunk->end_addr - chunk->start_addr + 1;
+
+ dma_free_coherent((struct device *) data, chunk_size,
+(void *) chunk->start_addr,
+(dma_addr_t) chunk->phys_addr);
+}
+
+void cio_gp_dma_destroy(struct gen_pool *gp_dma, struct device *dma_dev)
+{
+ if (!gp_dma)
+ return;
+ /* this is qite ugly but no better idea */
+ gen_pool_for_each_chunk(gp_dma, __gp_dma_free_dma, dma_dev);
+ gen_pool_destroy(gp_dma);
+}
+
+static void __init cio_dma_pool_init(void)
+{
+ /* No need to free up the resources: compiled in */
+ cio_dma_pool = cio_gp_dma_create(cio_get_dma_css_dev(), 1);
+}
+
+void *cio_gp_dma_zalloc(struct gen_pool *gp_dma, struct device *dma_dev,
+ size_t size)
+{
Re: [PATCH 05/10] s390/cio: introduce DMA pools to cio
On 20.05.19 14:13, Halil Pasic wrote: On Thu, 16 May 2019 15:59:22 +0200 (CEST) Sebastian Ott wrote: On Sun, 12 May 2019, Halil Pasic wrote: I've also got code that deals with AIRQ_IV_CACHELINE by turning the kmem_cache into a dma_pool. Cornelia, Sebastian which approach do you prefer: 1) get rid of cio_dma_pool and AIRQ_IV_CACHELINE, and waste a page per vector, or 2) go with the approach taken by the patch below? We only have a couple of users for airq_iv: virtio_ccw.c: 2K bits You mean a single allocation is 2k bits (VIRTIO_IV_BITS = 256 * 8)? My understanding is that the upper bound is more like: MAX_AIRQ_AREAS * VIRTIO_IV_BITS = 20 * 256 * 8 = 40960 bits. In practice it is most likely just 2k. pci with floating IRQs: <= 2K (for the per-function bit vectors) 1..4K (for the summary bit vector) As far as I can tell with virtio_pci arch_setup_msi_irqs() gets called once per device and allocates a small number of bits (2 and 3 in my test, it may depend on #virtqueues, but I did not check). So for an upper bound we would have to multiply with the upper bound of pci devices/functions. What is the upper bound on the number of functions? pci with CPU directed IRQs: 2K (for the per-CPU bit vectors) 1..nr_cpu (for the summary bit vector) I guess this is the same. The options are: * page allocations for everything Worst case we need 20 + #max_pci_dev pages. At the moment we allocate from ZONE_DMA (!) and waste a lot. * dma_pool for AIRQ_IV_CACHELINE ,gen_pool for others I prefer this. Explanation follows. * dma_pool for everything Less waste by factor factor 16. I think we should do option 3 and use a dma_pool with cachesize alignment for everything (as a prerequisite we have to limit config PCI_NR_FUNCTIONS to 2K - but that is not a real constraint). I prefer option 3 because it is conceptually the smallest change, and provides the behavior which is closest to the current one. Commit 414cbd1e3d14 "s390/airq: provide cacheline aligned ivs" (Sebastian Ott, 2019-02-27) could have been smaller had you implemented 'kmem_cache for everything' (and I would have had just to replace kmem_cache with dma_cache to achieve option 3). For some reason you decided to keep the iv->vector = kzalloc(size, GFP_KERNEL) code-path and make the client code request iv->vector = kmem_cache_zalloc(airq_iv_cache, GFP_KERNEL) explicitly, using a flag which you only decided to use for directed pci irqs AFAICT. My understanding of these decisions, and especially of the rationale behind commit 414cbd1e3d14 is limited. Thus if option 3 is the way to go, and the choices made by 414cbd1e3d14 were sub-optimal, I would feel much more comfortable if you provided a patch that revises and switches everything to kmem_chache. I would then just swap kmem_cache out with a dma_cache and my change would end up a straightforward and relatively clean one. So Sebastian, what shall we do? Regards, Halil Sebastian Folks, I had a version running with slight changes to the initial v1 patch set together with a revert of 414cbd1e3d14 "s390/airq: provide cacheline aligned ivs". That of course has the deficit of the memory usage pattern. Now you are discussing same substantial changes. The exercise was to get an initial working code through the door. We really need a decision! Michael ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/10] s390/mm: force swiotlb for protected virtualization
On 10.05.19 00:34, Halil Pasic wrote:
On Wed, 8 May 2019 15:15:40 +0200
Claudio Imbrenda wrote:
On Fri, 26 Apr 2019 20:32:39 +0200
Halil Pasic wrote:
On s390, protected virtualization guests have to use bounced I/O
buffers. That requires some plumbing.
Let us make sure, any device that uses DMA API with direct ops
correctly is spared from the problems, that a hypervisor attempting
I/O to a non-shared page would bring.
Signed-off-by: Halil Pasic
---
arch/s390/Kconfig | 4 +++
arch/s390/include/asm/mem_encrypt.h | 18 +
arch/s390/mm/init.c | 50
+ 3 files changed, 72
insertions(+) create mode 100644 arch/s390/include/asm/mem_encrypt.h
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 1c3fcf19c3af..5500d05d4d53 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -1,4 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
+config ARCH_HAS_MEM_ENCRYPT
+def_bool y
+
config MMU
def_bool y
@@ -191,6 +194,7 @@ config S390
select ARCH_HAS_SCALED_CPUTIME
select VIRT_TO_BUS
select HAVE_NMI
+ select SWIOTLB
config SCHED_OMIT_FRAME_POINTER
diff --git a/arch/s390/include/asm/mem_encrypt.h
b/arch/s390/include/asm/mem_encrypt.h new file mode 100644
index ..0898c09a888c
--- /dev/null
+++ b/arch/s390/include/asm/mem_encrypt.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef S390_MEM_ENCRYPT_H__
+#define S390_MEM_ENCRYPT_H__
+
+#ifndef __ASSEMBLY__
+
+#define sme_me_mask0ULL
This is rather ugly, but I understand why it's there
Nod.
+
+static inline bool sme_active(void) { return false; }
+extern bool sev_active(void);
+
+int set_memory_encrypted(unsigned long addr, int numpages);
+int set_memory_decrypted(unsigned long addr, int numpages);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* S390_MEM_ENCRYPT_H__ */
+
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 3e82f66d5c61..7e3cbd15dcfa 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -29,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -42,6 +44,8 @@
#include
#include
#include
+#include
+#include
pgd_t swapper_pg_dir[PTRS_PER_PGD] __section(.bss..swapper_pg_dir);
@@ -126,6 +130,50 @@ void mark_rodata_ro(void)
pr_info("Write protected read-only-after-init data: %luk\n",
size >> 10); }
+int set_memory_encrypted(unsigned long addr, int numpages)
+{
+ int i;
+
+ /* make all pages shared, (swiotlb, dma_free) */
this is a copypaste typo, I think? (should be UNshared?)
also, it doesn't make ALL pages unshared, but only those specified in
the parameters
Right a copy paste error. Needs correction. The all was meant like all
pages in the range specified by the arguments. But it is better changed
since it turned out to be confusing.
with this fixed:
Reviewed-by: Claudio Imbrenda
Thanks!
+ for (i = 0; i < numpages; ++i) {
+ uv_remove_shared(addr);
+ addr += PAGE_SIZE;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(set_memory_encrypted);
+
+int set_memory_decrypted(unsigned long addr, int numpages)
+{
+ int i;
+ /* make all pages shared (swiotlb, dma_alloca) */
same here with ALL
+ for (i = 0; i < numpages; ++i) {
+ uv_set_shared(addr);
+ addr += PAGE_SIZE;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(set_memory_decrypted);
+
+/* are we a protected virtualization guest? */
+bool sev_active(void)
this is also ugly. the correct solution would be probably to refactor
everything, including all the AMD SEV code let's not go there
Nod. Maybe later.
+{
+ return is_prot_virt_guest();
+}
+EXPORT_SYMBOL_GPL(sev_active);
+
+/* protected virtualization */
+static void pv_init(void)
+{
+ if (!sev_active())
can't you just use is_prot_virt_guest here?
Sure! I guess it would be less confusing. It is something I did not
remember to change when the interface for this provided by uv.h went
from sketchy to nice.
integrated in v2
Michael
Thanks again!
Regards,
Halil
+ return;
+
+ /* make sure bounce buffers are shared */
+ swiotlb_init(1);
+ swiotlb_update_mem_attributes();
+ swiotlb_force = SWIOTLB_FORCE;
+}
+
void __init mem_init(void)
{
cpumask_set_cpu(0, _mm.context.cpu_attach_mask);
@@ -134,6 +182,8 @@ void __init mem_init(void)
set_max_mapnr(max_low_pfn);
high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
+ pv_init();
+
/* Setup guest page hinting */
cmma_init();
--
Mit freundlichen Grüßen / Kind regards
Michael Müller
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der
Re: [PATCH 10/10] virtio/s390: make airq summary indicators DMA
On 08.05.19 17:11, Pierre Morel wrote:
On 26/04/2019 20:32, Halil Pasic wrote:
Hypervisor needs to interact with the summary indicators, so these
need to be DMA memory as well (at least for protected virtualization
guests).
Signed-off-by: Halil Pasic
---
drivers/s390/virtio/virtio_ccw.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/s390/virtio/virtio_ccw.c
b/drivers/s390/virtio/virtio_ccw.c
index 613b18001a0c..6058b07fea08 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -140,11 +140,17 @@ static int virtio_ccw_use_airq = 1;
struct airq_info {
rwlock_t lock;
- u8 summary_indicator;
+ u8 summary_indicator_idx;
struct airq_struct airq;
struct airq_iv *aiv;
};
static struct airq_info *airq_areas[MAX_AIRQ_AREAS];
+static u8 *summary_indicators;
+
+static inline u8 *get_summary_indicator(struct airq_info *info)
+{
+ return summary_indicators + info->summary_indicator_idx;
+}
#define CCW_CMD_SET_VQ 0x13
#define CCW_CMD_VDEV_RESET 0x33
@@ -225,7 +231,7 @@ static void virtio_airq_handler(struct airq_struct
*airq)
break;
vring_interrupt(0, (void *)airq_iv_get_ptr(info->aiv, ai));
}
- info->summary_indicator = 0;
+ *(get_summary_indicator(info)) = 0;
smp_wmb();
/* Walk through indicators field, summary indicator not active. */
for (ai = 0;;) {
@@ -237,7 +243,8 @@ static void virtio_airq_handler(struct airq_struct
*airq)
read_unlock(>lock);
}
-static struct airq_info *new_airq_info(void)
+/* call with airq_areas_lock held */
+static struct airq_info *new_airq_info(int index)
{
struct airq_info *info;
int rc;
@@ -252,7 +259,8 @@ static struct airq_info *new_airq_info(void)
return NULL;
}
info->airq.handler = virtio_airq_handler;
- info->airq.lsi_ptr = >summary_indicator;
+ info->summary_indicator_idx = index;
+ info->airq.lsi_ptr = get_summary_indicator(info);
info->airq.lsi_mask = 0xff;
info->airq.isc = VIRTIO_AIRQ_ISC;
rc = register_adapter_interrupt(>airq);
@@ -273,8 +281,9 @@ static unsigned long get_airq_indicator(struct
virtqueue *vqs[], int nvqs,
unsigned long bit, flags;
for (i = 0; i < MAX_AIRQ_AREAS && !indicator_addr; i++) {
+ /* TODO: this seems to be racy */
yes, my opinions too, was already racy, in my opinion, we need another
patch in another series to fix this.
However, not sure about the comment.
I will drop this comment for v2 of this patch series.
We shall fix the race with a separate patch.
Michael
if (!airq_areas[i])
- airq_areas[i] = new_airq_info();
+ airq_areas[i] = new_airq_info(i);
info = airq_areas[i];
if (!info)
return 0;
@@ -359,7 +368,7 @@ static void virtio_ccw_drop_indicator(struct
virtio_ccw_device *vcdev,
if (!thinint_area)
return;
thinint_area->summary_indicator =
- (unsigned long) _info->summary_indicator;
+ (unsigned long) get_summary_indicator(airq_info);
thinint_area->isc = VIRTIO_AIRQ_ISC;
ccw->cmd_code = CCW_CMD_SET_IND_ADAPTER;
ccw->count = sizeof(*thinint_area);
@@ -624,7 +633,7 @@ static int virtio_ccw_register_adapter_ind(struct
virtio_ccw_device *vcdev,
}
info = vcdev->airq_info;
thinint_area->summary_indicator =
- (unsigned long) >summary_indicator;
+ (unsigned long) get_summary_indicator(info);
thinint_area->isc = VIRTIO_AIRQ_ISC;
ccw->cmd_code = CCW_CMD_SET_IND_ADAPTER;
ccw->flags = CCW_FLAG_SLI;
@@ -1500,6 +1509,7 @@ static int __init virtio_ccw_init(void)
{
/* parse no_auto string before we do anything further */
no_auto_parse();
+ summary_indicators = cio_dma_zalloc(MAX_AIRQ_AREAS);
return ccw_driver_register(_ccw_driver);
}
device_initcall(virtio_ccw_init);
--
Mit freundlichen Grüßen / Kind regards
Michael Müller
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 10/10] virtio/s390: make airq summary indicators DMA
On 13.05.19 14:20, Cornelia Huck wrote:
On Fri, 26 Apr 2019 20:32:45 +0200
Halil Pasic wrote:
Hypervisor needs to interact with the summary indicators, so these
need to be DMA memory as well (at least for protected virtualization
guests).
Signed-off-by: Halil Pasic
---
drivers/s390/virtio/virtio_ccw.c | 24 +---
1 file changed, 17 insertions(+), 7 deletions(-)
(...)
@@ -237,7 +243,8 @@ static void virtio_airq_handler(struct airq_struct *airq)
read_unlock(>lock);
}
-static struct airq_info *new_airq_info(void)
+/* call with drivers/s390/virtio/virtio_ccw.cheld */
Hm, where is airq_areas_lock defined? If it was introduced in one of
the previous patches, I have missed it.
There is no airq_areas_lock defined currently. My assumption is that
this will be used in context with the likely race condition this
part of the patch is talking about.
@@ -273,8 +281,9 @@ static unsigned long get_airq_indicator(struct
virtqueue *vqs[], int nvqs,
unsigned long bit, flags;
for (i = 0; i < MAX_AIRQ_AREAS && !indicator_addr; i++) {
+ /* TODO: this seems to be racy */
if (!airq_areas[i])
- airq_areas[i] = new_airq_info();
+ airq_areas[i] = new_airq_info(i);
As this shall be handled by a separate patch I will drop the comment
in regard to airq_areas_lock from this patch as well for v2.
Michael
+static struct airq_info *new_airq_info(int index)
{
struct airq_info *info;
int rc;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/10] s390/mm: force swiotlb for protected virtualization
On 29.04.19 16:05, Christian Borntraeger wrote: On 29.04.19 15:59, Halil Pasic wrote: On Fri, 26 Apr 2019 12:27:11 -0700 Christoph Hellwig wrote: On Fri, Apr 26, 2019 at 08:32:39PM +0200, Halil Pasic wrote: +EXPORT_SYMBOL_GPL(set_memory_encrypted); +EXPORT_SYMBOL_GPL(set_memory_decrypted); +EXPORT_SYMBOL_GPL(sev_active); Why do you export these? I know x86 exports those as well, but it shoudn't be needed there either. I export these to be in line with the x86 implementation (which is the original and seems to be the only one at the moment). I assumed that 'exported or not' is kind of a part of the interface definition. Honestly, I did not give it too much thought. For x86 set_memory(en|de)crypted got exported by 95cf9264d5f3 "x86, drm, fbdev: Do not specify encrypted memory for video mappings" (Tom Lendacky, 2017-07-17). With CONFIG_FB_VGA16=m seems to be necessary for x84. If the consensus is don't export: I won't. I'm fine one way or the other. @Christian, what is your take on this? If we do not need it today for anything (e.g. virtio-gpu) then we can get rid of the exports (and introduce them when necessary). I'll take them out then. Thank you very much! Regards, Halil ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 01/10] virtio/s390: use vring_create_virtqueue
On 13.05.19 11:52, Cornelia Huck wrote:
On Sun, 12 May 2019 12:47:39 -0400
"Michael S. Tsirkin" wrote:
On Fri, May 10, 2019 at 04:07:44PM +0200, Cornelia Huck wrote:
On Tue, 7 May 2019 15:58:12 +0200
Christian Borntraeger wrote:
On 05.05.19 13:15, Cornelia Huck wrote:
On Sat, 4 May 2019 16:03:40 +0200
Halil Pasic wrote:
On Fri, 3 May 2019 16:04:48 -0400
"Michael S. Tsirkin" wrote:
On Fri, May 03, 2019 at 11:17:24AM +0200, Cornelia Huck wrote:
On Fri, 26 Apr 2019 20:32:36 +0200
Halil Pasic wrote:
The commit 2a2d1382fe9d ("virtio: Add improved queue allocation API")
establishes a new way of allocating virtqueues (as a part of the effort
that taught DMA to virtio rings).
In the future we will want virtio-ccw to use the DMA API as well.
Let us switch from the legacy method of allocating virtqueues to
vring_create_virtqueue() as the first step into that direction.
Signed-off-by: Halil Pasic
---
drivers/s390/virtio/virtio_ccw.c | 30 +++---
1 file changed, 11 insertions(+), 19 deletions(-)
Reviewed-by: Cornelia Huck
I'd vote for merging this patch right away for 5.2.
So which tree is this going through? mine?
Christian, what do you think? If the whole series is supposed to go in
in one go (which I hope it is), via Martin's tree could be the simplest
route IMHO.
The first three patches are virtio(-ccw) only and the those are the ones
that I think are ready to go.
I'm not feeling comfortable going forward with the remainder as it
stands now; waiting for some other folks to give feedback. (They are
touching/interacting with code parts I'm not so familiar with, and lack
of documentation, while not the developers' fault, does not make it
easier.)
Michael, would you like to pick up 1-3 for your tree directly? That
looks like the easiest way.
Agreed. Michael please pick 1-3.
We will continue to review 4- first and then see which tree is best.
Michael, please let me know if you'll pick directly or whether I should
post a series.
[Given that the patches are from one virtio-ccw maintainer and reviewed
by the other, picking directly would eliminate an unnecessary
indirection :)]
picked them
Thanks!
Connie,
if I get you right here, you don't need a v2 for the
patches 1 through 3?
Thanks,
Michael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v1] vhost: avoid large order allocations
On Tue, 13 May 2014 18:15:27 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Tue, May 13, 2014 at 04:29:58PM +0200, Romain Francoise wrote: Michael S. Tsirkin m...@redhat.com writes: Please dont' do this, extra indirection hurts performance. Instead, please change vhost_net_open and scsi to allocate the whole structure with vmalloc if kmalloc fails, along the lines of 74d332c13b2148ae934ea94dac1745ae92efe8e5 Back in January 2013, you didn't seem to think it was a good idea: https://lkml.org/lkml/2013/1/23/492 Hmm true, and Dave thought the structure's too large. I'll have to do some benchmarks to see what the effect of Michael's patch is, performance-wise. If it's too expensive I can pick up your patch, no need to repost. Hi Michael, do you have any update in this case for us? Michael ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v1] vhost: avoid large order allocations
From: Dong Dong Chen ddon...@cn.ibm.com
Under memory pressure we observe load issues with module vhost_net, as the
vhost setup code will try to do an order 4 allocation for the device.
The likeliness of this issue to occur can be reduced when the statically
allocated variable iov in struct vhost_virtqueue is dynamically allocated
with exactly the required size, reducing this to an order 2 allocation.
Signed-off-by: Dong Dong Chen ddon...@cn.ibm.com
Reviewed-by: Christian Borntraeger borntrae...@de.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Reviewed-by: Michael Mueller m...@linux.vnet.ibm.com
---
drivers/vhost/net.c | 4 ++--
drivers/vhost/scsi.c | 4 ++--
drivers/vhost/test.c | 2 +-
drivers/vhost/vhost.c | 6 +-
drivers/vhost/vhost.h | 2 +-
5 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index be414d2..e3a9a68 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -374,7 +374,7 @@ static void handle_tx(struct vhost_net *net)
break;
head = vhost_get_vq_desc(net-dev, vq, vq-iov,
-ARRAY_SIZE(vq-iov),
+UIO_MAXIOV,
out, in,
NULL, NULL);
/* On error, stop handling until the next kick. */
@@ -506,7 +506,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
goto err;
}
r = vhost_get_vq_desc(vq-dev, vq, vq-iov + seg,
- ARRAY_SIZE(vq-iov) - seg, out,
+ UIO_MAXIOV - seg, out,
in, log, log_num);
if (unlikely(r 0))
goto err;
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index cf50ce9..a70f1d9 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -607,7 +607,7 @@ tcm_vhost_do_evt_work(struct vhost_scsi *vs, struct
tcm_vhost_evt *evt)
again:
vhost_disable_notify(vs-dev, vq);
head = vhost_get_vq_desc(vs-dev, vq, vq-iov,
- ARRAY_SIZE(vq-iov), out, in,
+ UIO_MAXIOV, out, in,
NULL, NULL);
if (head 0) {
vs-vs_events_missed = true;
@@ -946,7 +946,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct
vhost_virtqueue *vq)
for (;;) {
head = vhost_get_vq_desc(vs-dev, vq, vq-iov,
- ARRAY_SIZE(vq-iov), out, in,
+ UIO_MAXIOV, out, in,
NULL, NULL);
pr_debug(vhost_get_vq_desc: head: %d, out: %u in: %u\n,
head, out, in);
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index c2a54fb..2e01920 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -54,7 +54,7 @@ static void handle_vq(struct vhost_test *n)
for (;;) {
head = vhost_get_vq_desc(n-dev, vq, vq-iov,
-ARRAY_SIZE(vq-iov),
+UIO_MAXIOV,
out, in,
NULL, NULL);
/* On error, stop handling until the next kick. */
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 78987e4..9017a55 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -251,6 +251,8 @@ static int vhost_worker(void *data)
static void vhost_vq_free_iovecs(struct vhost_virtqueue *vq)
{
+ kfree(vq-iov);
+ vq-iov = NULL;
kfree(vq-indirect);
vq-indirect = NULL;
kfree(vq-log);
@@ -267,11 +269,12 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
for (i = 0; i dev-nvqs; ++i) {
vq = dev-vqs[i];
+ vq-iov = kmalloc(sizeof(*vq-iov) * UIO_MAXIOV, GFP_KERNEL);
vq-indirect = kmalloc(sizeof *vq-indirect * UIO_MAXIOV,
GFP_KERNEL);
vq-log = kmalloc(sizeof *vq-log * UIO_MAXIOV, GFP_KERNEL);
vq-heads = kmalloc(sizeof *vq-heads * UIO_MAXIOV, GFP_KERNEL);
- if (!vq-indirect || !vq-log || !vq-heads)
+ if (!vq-iov || !vq-indirect || !vq-log || !vq-heads)
goto err_nomem;
}
return 0;
@@ -310,6 +313,7 @@ void vhost_dev_init(struct vhost_dev *dev,
for (i = 0; i dev-nvqs; ++i) {
vq = dev-vqs[i];
vq-log = NULL;
+ vq-iov = NULL;
vq-indirect = NULL;
vq-heads = NULL;
vq-dev = dev;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 35eeb2a..541f757 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
[PATCH v1] vhost: avoid large order allocations
A test case which generates memory pressure while performing guest administration fails with vhost triggering page allocation failure and guest not starting up. After some analysis we discovered the allocation order of vhost to be rensponsible for this behaviour. Thus we suggest patch 1/1 which dynamically allocates the required memory. Please see its description for details. Thanks, Michael Dong Dong Chen (1): vhost: avoid large order allocations drivers/vhost/net.c | 4 ++-- drivers/vhost/scsi.c | 4 ++-- drivers/vhost/test.c | 2 +- drivers/vhost/vhost.c | 6 +- drivers/vhost/vhost.h | 2 +- 5 files changed, 11 insertions(+), 7 deletions(-) -- 1.8.3.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v1] vhost: avoid large order allocations
On Tue, 13 May 2014 11:40:49 +0300 Michael S. Tsirkin m...@redhat.com wrote: Please dont' do this, extra indirection hurts performance. Instead, please change vhost_net_open and scsi to allocate the whole structure with vmalloc if kmalloc fails, along the lines of 74d332c13b2148ae934ea94dac1745ae92efe8e5 Thanks for pointing us to: net: extend net_device allocation to vmalloc() We'll try to adapt. Michael ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v1] vhost: avoid large order allocations
On Tue, 13 May 2014 18:15:27 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Tue, May 13, 2014 at 04:29:58PM +0200, Romain Francoise wrote: Michael S. Tsirkin m...@redhat.com writes: Please dont' do this, extra indirection hurts performance. Instead, please change vhost_net_open and scsi to allocate the whole structure with vmalloc if kmalloc fails, along the lines of 74d332c13b2148ae934ea94dac1745ae92efe8e5 Back in January 2013, you didn't seem to think it was a good idea: https://lkml.org/lkml/2013/1/23/492 Hmm true, and Dave thought the structure's too large. I'll have to do some benchmarks to see what the effect of Michael's patch is, performance-wise. If it's too expensive I can pick up your patch, no need to repost. Thanks, let us know then. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v1] vhost: avoid large order allocations
On Tue, 13 May 2014 16:29:58 +0200 Romain Francoise rom...@orebokech.com wrote: Michael S. Tsirkin m...@redhat.com writes: Please dont' do this, extra indirection hurts performance. Instead, please change vhost_net_open and scsi to allocate the whole structure with vmalloc if kmalloc fails, along the lines of 74d332c13b2148ae934ea94dac1745ae92efe8e5 Back in January 2013, you didn't seem to think it was a good idea: https://lkml.org/lkml/2013/1/23/492 Hi Romain, in that case I'd suggest that you submit your patch, ours will look pretty much the same! Cheers Michael ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
