[PATCH 1/3] S390: take a full byte as ext_param indicator

[PATCH 1/3] S390: take a full byte as ext_param indicator

[Alexander Graf]

Currenty the ext_param field only distinguishes between "config change" and
"vring interrupt". We can do a lot more with it though, so let's enable a
full byte of possible values and constants to #defines while at it.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - move defines to virtio_s390.h
---
 arch/s390/include/asm/kvm_virtio.h |    6 ++++++
 drivers/s390/kvm/kvm_virtio.c      |   19 +++++++++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/s390/include/asm/kvm_virtio.h b/arch/s390/include/asm/kvm_virtio.h
index acdfdff..3f5d100 100644
--- a/arch/s390/include/asm/kvm_virtio.h
+++ b/arch/s390/include/asm/kvm_virtio.h
@@ -54,4 +54,10 @@ struct kvm_vqconfig {
  * This is pagesize for historical reasons. */
 #define KVM_S390_VIRTIO_RING_ALIGN	4096
 
+
+/* These values are supposed to be in ext_params on an interrupt */
+#define VIRTIO_PARAM_MASK		0xff
+#define VIRTIO_PARAM_VRING_INTERRUPT	0x0
+#define VIRTIO_PARAM_CONFIG_CHANGED	0x1
+
 #endif
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 4e298bc..68cef4d 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -334,7 +334,7 @@ static void kvm_extint_handler(u16 code)
 {
 	struct virtqueue *vq;
 	u16 subcode;
-	int config_changed;
+	u32 param;
 
 	subcode = S390_lowcore.cpu_addr;
 	if ((subcode & 0xff00) != VIRTIO_SUBCODE_64)
@@ -343,18 +343,25 @@ static void kvm_extint_handler(u16 code)
 	/* The LSB might be overloaded, we have to mask it */
 	vq = (struct virtqueue *)(S390_lowcore.ext_params2 & ~1UL);
 
-	/* We use the LSB of extparam, to decide, if this interrupt is a config
-	 * change or a "standard" interrupt */
-	config_changed = S390_lowcore.ext_params & 1;
+	/* We use ext_params to decide what this interrupt means */
+	param = S390_lowcore.ext_params & VIRTIO_PARAM_MASK;
 
-	if (config_changed) {
+	switch (param) {
+	case VIRTIO_PARAM_CONFIG_CHANGED:
+	{
 		struct virtio_driver *drv;
 		drv = container_of(vq->vdev->dev.driver,
 				   struct virtio_driver, driver);
 		if (drv->config_changed)
 			drv->config_changed(vq->vdev);
-	} else
+
+		break;
+	}
+	case VIRTIO_PARAM_VRING_INTERRUPT:
+	default:
 		vring_interrupt(0, vq);
+		break;
+	}
 }
 
 /*
-- 
1.6.0.2

[PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

The one big missing feature in s390-virtio was hotplugging. This is no more.
This patch implements hotplug add support, so you can on the fly add new devices
in the guest.

Keep in mind that this needs a patch for qemu to actually leverage the
functionality.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - move dev_add to kvm_virtio.h
---
 arch/s390/include/asm/kvm_virtio.h |    1 +
 drivers/s390/kvm/kvm_virtio.c      |   47 ++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/arch/s390/include/asm/kvm_virtio.h b/arch/s390/include/asm/kvm_virtio.h
index 3f5d100..72f6141 100644
--- a/arch/s390/include/asm/kvm_virtio.h
+++ b/arch/s390/include/asm/kvm_virtio.h
@@ -59,5 +59,6 @@ struct kvm_vqconfig {
 #define VIRTIO_PARAM_MASK		0xff
 #define VIRTIO_PARAM_VRING_INTERRUPT	0x0
 #define VIRTIO_PARAM_CONFIG_CHANGED	0x1
+#define VIRTIO_PARAM_DEV_ADD		0x2
 
 #endif
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 68cef4d..5a46b8c 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -32,6 +32,7 @@
  * The pointer to our (page) of device descriptions.
  */
 static void *kvm_devices;
+struct work_struct hotplug_work;
 
 struct kvm_device {
 	struct virtio_device vdev;
@@ -328,6 +329,47 @@ static void scan_devices(void)
 }
 
 /*
+ * match for a kvm device with a specific desc pointer
+ */
+static int match_desc(struct device *dev, void *data)
+{
+	if ((ulong)to_kvmdev(dev_to_virtio(dev))->desc == (ulong)data)
+		return 1;
+
+	return 0;
+}
+
+/*
+ * hotplug_device tries to find changes in the device page.
+ */
+static void hotplug_devices(struct work_struct *dummy)
+{
+	unsigned int i;
+	struct kvm_device_desc *d;
+	struct device *dev;
+
+	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
+		d = kvm_devices + i;
+
+		/* end of list */
+		if (d->type == 0)
+			break;
+
+		/* device already exists */
+		dev = device_find_child(kvm_root, d, match_desc);
+		if (dev) {
+			/* XXX check for hotplug remove */
+			put_device(dev);
+			continue;
+		}
+
+		/* new device */
+		printk(KERN_INFO "Adding new virtio device %p\n", d);
+		add_kvm_device(d, i);
+	}
+}
+
+/*
  * we emulate the request_irq behaviour on top of s390 extints
  */
 static void kvm_extint_handler(u16 code)
@@ -357,6 +399,9 @@ static void kvm_extint_handler(u16 code)
 
 		break;
 	}
+	case VIRTIO_PARAM_DEV_ADD:
+		schedule_work(&hotplug_work);
+		break;
 	case VIRTIO_PARAM_VRING_INTERRUPT:
 	default:
 		vring_interrupt(0, vq);
@@ -390,6 +435,8 @@ static int __init kvm_devices_init(void)
 
 	kvm_devices = (void *) real_memory_size;
 
+	INIT_WORK(&hotplug_work, hotplug_devices);
+
 	ctl_set_bit(0, 9);
 	register_external_interrupt(0x2603, kvm_extint_handler);
 
-- 
1.6.0.2

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Heiko Carstens]

On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
> +static void hotplug_devices(struct work_struct *dummy)
> +{
> +	unsigned int i;
> +	struct kvm_device_desc *d;
> +	struct device *dev;
> +
> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {

This should be 

	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {

otherwise you might have memory accesses beyond the device page...

> +		d = kvm_devices + i;
> +
> +		/* end of list */
> +		if (d->type == 0)
> +			break;

...even if that should not happen if everything works.
But let's be paranoid.

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

On 25.08.2010, at 10:16, Heiko Carstens wrote:

> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
>> +static void hotplug_devices(struct work_struct *dummy)
>> +{
>> +	unsigned int i;
>> +	struct kvm_device_desc *d;
>> +	struct device *dev;
>> +
>> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
> 
> This should be 
> 
> 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
> 
> otherwise you might have memory accesses beyond the device page...

Oh, this is a simple copy&paste from the original search method. Is d valid in the first part of the loop already?

> 
>> +		d = kvm_devices + i;
>> +
>> +		/* end of list */
>> +		if (d->type == 0)
>> +			break;
> 
> ...even if that should not happen if everything works.
> But let's be paranoid.

Yeah :). I like paranoid.


Alex

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

On 25.08.2010, at 10:16, Heiko Carstens wrote:

> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
>> +static void hotplug_devices(struct work_struct *dummy)
>> +{
>> +	unsigned int i;
>> +	struct kvm_device_desc *d;
>> +	struct device *dev;
>> +
>> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
> 
> This should be 
> 
> 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
> 
> otherwise you might have memory accesses beyond the device page...

Oh, this is a simple copy&paste from the original search method. Is d valid in the first part of the loop already?

> 
>> +		d = kvm_devices + i;
>> +
>> +		/* end of list */
>> +		if (d->type == 0)
>> +			break;
> 
> ...even if that should not happen if everything works.
> But let's be paranoid.

Yeah :). I like paranoid.


Alex

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Heiko Carstens]

On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
> On 25.08.2010, at 10:16, Heiko Carstens wrote:
> > On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
> >> +static void hotplug_devices(struct work_struct *dummy)
> >> +{
> >> +	unsigned int i;
> >> +	struct kvm_device_desc *d;
> >> +	struct device *dev;
> >> +
> >> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
> > 
> > This should be 
> > 
> > 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
> > 
> > otherwise you might have memory accesses beyond the device page...
> 
> Oh, this is a simple copy&paste from the original search method.
> Is d valid in the first part of the loop already?

No, you would need to initialize it with kvm_devices if you change
the loop.

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Heiko Carstens]

On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
> On 25.08.2010, at 10:16, Heiko Carstens wrote:
> > On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
> >> +static void hotplug_devices(struct work_struct *dummy)
> >> +{
> >> +	unsigned int i;
> >> +	struct kvm_device_desc *d;
> >> +	struct device *dev;
> >> +
> >> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
> > 
> > This should be 
> > 
> > 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
> > 
> > otherwise you might have memory accesses beyond the device page...
> 
> Oh, this is a simple copy&paste from the original search method.
> Is d valid in the first part of the loop already?

No, you would need to initialize it with kvm_devices if you change
the loop.

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

On 25.08.2010, at 10:35, Heiko Carstens wrote:

> On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
>> On 25.08.2010, at 10:16, Heiko Carstens wrote:
>>> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
>>>> +static void hotplug_devices(struct work_struct *dummy)
>>>> +{
>>>> +	unsigned int i;
>>>> +	struct kvm_device_desc *d;
>>>> +	struct device *dev;
>>>> +
>>>> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
>>> 
>>> This should be 
>>> 
>>> 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
>>> 
>>> otherwise you might have memory accesses beyond the device page...
>> 
>> Oh, this is a simple copy&paste from the original search method.
>> Is d valid in the first part of the loop already?
> 
> No, you would need to initialize it with kvm_devices if you change
> the loop.

But even then it would take the size of the current entry, not the next one we're actually looking for, no? Maybe it'd be easier to just convert this into a while loop and explicitly open code everything.

Alex

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

On 25.08.2010, at 10:35, Heiko Carstens wrote:

> On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
>> On 25.08.2010, at 10:16, Heiko Carstens wrote:
>>> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
>>>> +static void hotplug_devices(struct work_struct *dummy)
>>>> +{
>>>> +	unsigned int i;
>>>> +	struct kvm_device_desc *d;
>>>> +	struct device *dev;
>>>> +
>>>> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
>>> 
>>> This should be 
>>> 
>>> 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
>>> 
>>> otherwise you might have memory accesses beyond the device page...
>> 
>> Oh, this is a simple copy&paste from the original search method.
>> Is d valid in the first part of the loop already?
> 
> No, you would need to initialize it with kvm_devices if you change
> the loop.

But even then it would take the size of the current entry, not the next one we're actually looking for, no? Maybe it'd be easier to just convert this into a while loop and explicitly open code everything.

Alex

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Heiko Carstens]

On Wed, Aug 25, 2010 at 10:34:29AM +0200, Alexander Graf wrote:
> On 25.08.2010, at 10:35, Heiko Carstens wrote:
> > On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
> >> On 25.08.2010, at 10:16, Heiko Carstens wrote:
> >>> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
> >>>> +static void hotplug_devices(struct work_struct *dummy)
> >>>> +{
> >>>> +	unsigned int i;
> >>>> +	struct kvm_device_desc *d;
> >>>> +	struct device *dev;
> >>>> +
> >>>> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
> >>> 
> >>> This should be 
> >>> 
> >>> 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
> >>> 
> >>> otherwise you might have memory accesses beyond the device page...
> >> 
> >> Oh, this is a simple copy&paste from the original search method.
> >> Is d valid in the first part of the loop already?
> > 
> > No, you would need to initialize it with kvm_devices if you change
> > the loop.
> 
> But even then it would take the size of the current entry, not the next
> one we're actually looking for, no? Maybe it'd be easier to just convert
> this into a while loop and explicitly open code everything.

Yes indeed. It's going to be quite ugly if you want to make sure that at
no time you're going to access memory beyond the device page. Eeek.. :)

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Heiko Carstens]

On Wed, Aug 25, 2010 at 10:34:29AM +0200, Alexander Graf wrote:
> On 25.08.2010, at 10:35, Heiko Carstens wrote:
> > On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
> >> On 25.08.2010, at 10:16, Heiko Carstens wrote:
> >>> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
> >>>> +static void hotplug_devices(struct work_struct *dummy)
> >>>> +{
> >>>> +	unsigned int i;
> >>>> +	struct kvm_device_desc *d;
> >>>> +	struct device *dev;
> >>>> +
> >>>> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
> >>> 
> >>> This should be 
> >>> 
> >>> 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
> >>> 
> >>> otherwise you might have memory accesses beyond the device page...
> >> 
> >> Oh, this is a simple copy&paste from the original search method.
> >> Is d valid in the first part of the loop already?
> > 
> > No, you would need to initialize it with kvm_devices if you change
> > the loop.
> 
> But even then it would take the size of the current entry, not the next
> one we're actually looking for, no? Maybe it'd be easier to just convert
> this into a while loop and explicitly open code everything.

Yes indeed. It's going to be quite ugly if you want to make sure that at
no time you're going to access memory beyond the device page. Eeek.. :)

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

On 25.08.2010, at 10:48, Heiko Carstens wrote:

> On Wed, Aug 25, 2010 at 10:34:29AM +0200, Alexander Graf wrote:
>> On 25.08.2010, at 10:35, Heiko Carstens wrote:
>>> On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
>>>> On 25.08.2010, at 10:16, Heiko Carstens wrote:
>>>>> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
>>>>>> +static void hotplug_devices(struct work_struct *dummy)
>>>>>> +{
>>>>>> +	unsigned int i;
>>>>>> +	struct kvm_device_desc *d;
>>>>>> +	struct device *dev;
>>>>>> +
>>>>>> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
>>>>> 
>>>>> This should be 
>>>>> 
>>>>> 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
>>>>> 
>>>>> otherwise you might have memory accesses beyond the device page...
>>>> 
>>>> Oh, this is a simple copy&paste from the original search method.
>>>> Is d valid in the first part of the loop already?
>>> 
>>> No, you would need to initialize it with kvm_devices if you change
>>> the loop.
>> 
>> But even then it would take the size of the current entry, not the next
>> one we're actually looking for, no? Maybe it'd be easier to just convert
>> this into a while loop and explicitly open code everything.
> 
> Yes indeed. It's going to be quite ugly if you want to make sure that at
> no time you're going to access memory beyond the device page. Eeek.. :)

Thinking about this a bit more ... it's no problem to access beyond the device page. After the device page follow the rings and we always have rings because we always have a virtio console :).


Alex

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

On 25.08.2010, at 10:48, Heiko Carstens wrote:

> On Wed, Aug 25, 2010 at 10:34:29AM +0200, Alexander Graf wrote:
>> On 25.08.2010, at 10:35, Heiko Carstens wrote:
>>> On Wed, Aug 25, 2010 at 10:20:03AM +0200, Alexander Graf wrote:
>>>> On 25.08.2010, at 10:16, Heiko Carstens wrote:
>>>>> On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
>>>>>> +static void hotplug_devices(struct work_struct *dummy)
>>>>>> +{
>>>>>> +	unsigned int i;
>>>>>> +	struct kvm_device_desc *d;
>>>>>> +	struct device *dev;
>>>>>> +
>>>>>> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {
>>>>> 
>>>>> This should be 
>>>>> 
>>>>> 	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {
>>>>> 
>>>>> otherwise you might have memory accesses beyond the device page...
>>>> 
>>>> Oh, this is a simple copy&paste from the original search method.
>>>> Is d valid in the first part of the loop already?
>>> 
>>> No, you would need to initialize it with kvm_devices if you change
>>> the loop.
>> 
>> But even then it would take the size of the current entry, not the next
>> one we're actually looking for, no? Maybe it'd be easier to just convert
>> this into a while loop and explicitly open code everything.
> 
> Yes indeed. It's going to be quite ugly if you want to make sure that at
> no time you're going to access memory beyond the device page. Eeek.. :)

Thinking about this a bit more ... it's no problem to access beyond the device page. After the device page follow the rings and we always have rings because we always have a virtio console :).


Alex

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Heiko Carstens]

On Tue, Aug 24, 2010 at 03:48:51PM +0200, Alexander Graf wrote:
> +static void hotplug_devices(struct work_struct *dummy)
> +{
> +	unsigned int i;
> +	struct kvm_device_desc *d;
> +	struct device *dev;
> +
> +	for (i = 0; i < PAGE_SIZE; i += desc_size(d)) {

This should be 

	for (i = 0; i + desc_size(d) <= PAGE_SIZE; i += desc_size(d)) {

otherwise you might have memory accesses beyond the device page...

> +		d = kvm_devices + i;
> +
> +		/* end of list */
> +		if (d->type == 0)
> +			break;

...even if that should not happen if everything works.
But let's be paranoid.

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

On 24.08.2010, at 15:48, Alexander Graf wrote:

> The one big missing feature in s390-virtio was hotplugging. This is no more.
> This patch implements hotplug add support, so you can on the fly add new devices
> in the guest.
> 
> Keep in mind that this needs a patch for qemu to actually leverage the
> functionality.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

ping (on the patch set)?


Alex

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Alexander Graf]

On 24.08.2010, at 15:48, Alexander Graf wrote:

> The one big missing feature in s390-virtio was hotplugging. This is no more.
> This patch implements hotplug add support, so you can on the fly add new devices
> in the guest.
> 
> Keep in mind that this needs a patch for qemu to actually leverage the
> functionality.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

ping (on the patch set)?


Alex

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Avi Kivity]

  On 09/12/2010 02:42 AM, Alexander Graf wrote:
> On 24.08.2010, at 15:48, Alexander Graf wrote:
>
>> The one big missing feature in s390-virtio was hotplugging. This is no more.
>> This patch implements hotplug add support, so you can on the fly add new devices
>> in the guest.
>>
>> Keep in mind that this needs a patch for qemu to actually leverage the
>> functionality.
>>
>> Signed-off-by: Alexander Graf<agraf@suse.de>
> ping (on the patch set)?
>

Actually Marcelo applied it.  But the natural place for it is Rusty's 
virtio tree.  Rusty, if you want to take it, let me know and I'll drop 
it from kvm.git.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Rusty Russell]

On Sun, 12 Sep 2010 06:30:43 pm Avi Kivity wrote:
>   On 09/12/2010 02:42 AM, Alexander Graf wrote:
> > On 24.08.2010, at 15:48, Alexander Graf wrote:
> >
> >> The one big missing feature in s390-virtio was hotplugging. This is no more.
> >> This patch implements hotplug add support, so you can on the fly add new devices
> >> in the guest.
> >>
> >> Keep in mind that this needs a patch for qemu to actually leverage the
> >> functionality.
> >>
> >> Signed-off-by: Alexander Graf<agraf@suse.de>
> > ping (on the patch set)?
> >
> 
> Actually Marcelo applied it.  But the natural place for it is Rusty's 
> virtio tree.  Rusty, if you want to take it, let me know and I'll drop 
> it from kvm.git.

I thought it would be in the s390 tree, which is why I didn't take it...

But I'm *always* happy to let do the work!

Thanks,
Rusty.

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Martin Schwidefsky]

On Mon, 13 Sep 2010 13:05:57 +0930
Rusty Russell <rusty@rustcorp.com.au> wrote:

> On Sun, 12 Sep 2010 06:30:43 pm Avi Kivity wrote:
> >   On 09/12/2010 02:42 AM, Alexander Graf wrote:
> > > On 24.08.2010, at 15:48, Alexander Graf wrote:
> > >
> > >> The one big missing feature in s390-virtio was hotplugging. This is no more.
> > >> This patch implements hotplug add support, so you can on the fly add new devices
> > >> in the guest.
> > >>
> > >> Keep in mind that this needs a patch for qemu to actually leverage the
> > >> functionality.
> > >>
> > >> Signed-off-by: Alexander Graf<agraf@suse.de>
> > > ping (on the patch set)?
> > >
> > 
> > Actually Marcelo applied it.  But the natural place for it is Rusty's 
> > virtio tree.  Rusty, if you want to take it, let me know and I'll drop 
> > it from kvm.git.
> 
> I thought it would be in the s390 tree, which is why I didn't take it...
> 
> But I'm *always* happy to let do the work!

I didn't pick them up after I saw that Marcelo took them. If others want
to do the work, be my guest..

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Martin Schwidefsky]

On Mon, 13 Sep 2010 13:05:57 +0930
Rusty Russell <rusty@rustcorp.com.au> wrote:

> On Sun, 12 Sep 2010 06:30:43 pm Avi Kivity wrote:
> >   On 09/12/2010 02:42 AM, Alexander Graf wrote:
> > > On 24.08.2010, at 15:48, Alexander Graf wrote:
> > >
> > >> The one big missing feature in s390-virtio was hotplugging. This is no more.
> > >> This patch implements hotplug add support, so you can on the fly add new devices
> > >> in the guest.
> > >>
> > >> Keep in mind that this needs a patch for qemu to actually leverage the
> > >> functionality.
> > >>
> > >> Signed-off-by: Alexander Graf<agraf@suse.de>
> > > ping (on the patch set)?
> > >
> > 
> > Actually Marcelo applied it.  But the natural place for it is Rusty's 
> > virtio tree.  Rusty, if you want to take it, let me know and I'll drop 
> > it from kvm.git.
> 
> I thought it would be in the s390 tree, which is why I didn't take it...
> 
> But I'm *always* happy to let do the work!

I didn't pick them up after I saw that Marcelo took them. If others want
to do the work, be my guest..

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Avi Kivity]

  On 09/13/2010 09:41 AM, Martin Schwidefsky wrote:
>>> Actually Marcelo applied it.  But the natural place for it is Rusty's
>>> virtio tree.  Rusty, if you want to take it, let me know and I'll drop
>>> it from kvm.git.
>> I thought it would be in the s390 tree, which is why I didn't take it...
>>
>> But I'm *always* happy to let do the work!
> I didn't pick them up after I saw that Marcelo took them. If others want
> to do the work, be my guest..
>

I just hope that all this generosity doesn't lead to merge conflicts 
later, or people basing their stuff on stale code.  But it isn't like 
this is a high churn area.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Avi Kivity]

  On 09/13/2010 09:41 AM, Martin Schwidefsky wrote:
>>> Actually Marcelo applied it.  But the natural place for it is Rusty's
>>> virtio tree.  Rusty, if you want to take it, let me know and I'll drop
>>> it from kvm.git.
>> I thought it would be in the s390 tree, which is why I didn't take it...
>>
>> But I'm *always* happy to let do the work!
> I didn't pick them up after I saw that Marcelo took them. If others want
> to do the work, be my guest..
>

I just hope that all this generosity doesn't lead to merge conflicts 
later, or people basing their stuff on stale code.  But it isn't like 
this is a high churn area.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Rusty Russell]

On Sun, 12 Sep 2010 06:30:43 pm Avi Kivity wrote:
>   On 09/12/2010 02:42 AM, Alexander Graf wrote:
> > On 24.08.2010, at 15:48, Alexander Graf wrote:
> >
> >> The one big missing feature in s390-virtio was hotplugging. This is no more.
> >> This patch implements hotplug add support, so you can on the fly add new devices
> >> in the guest.
> >>
> >> Keep in mind that this needs a patch for qemu to actually leverage the
> >> functionality.
> >>
> >> Signed-off-by: Alexander Graf<agraf@suse.de>
> > ping (on the patch set)?
> >
> 
> Actually Marcelo applied it.  But the natural place for it is Rusty's 
> virtio tree.  Rusty, if you want to take it, let me know and I'll drop 
> it from kvm.git.

I thought it would be in the s390 tree, which is why I didn't take it...

But I'm *always* happy to let do the work!

Thanks,
Rusty.

Re: [PATCH 2/3] S390: Add virtio hotplug add support

[Avi Kivity]

  On 09/12/2010 02:42 AM, Alexander Graf wrote:
> On 24.08.2010, at 15:48, Alexander Graf wrote:
>
>> The one big missing feature in s390-virtio was hotplugging. This is no more.
>> This patch implements hotplug add support, so you can on the fly add new devices
>> in the guest.
>>
>> Keep in mind that this needs a patch for qemu to actually leverage the
>> functionality.
>>
>> Signed-off-by: Alexander Graf<agraf@suse.de>
> ping (on the patch set)?
>

Actually Marcelo applied it.  But the natural place for it is Rusty's 
virtio tree.  Rusty, if you want to take it, let me know and I'll drop 
it from kvm.git.

-- 
error compiling committee.c: too many arguments to function

[PATCH 3/3] S390: Export kvm_virtio.h

[Alexander Graf]

As suggested by Christian, we should expose headers to user space with
information that might be valuable there. The s390 virtio interface is
one of those cases. It defines an ABI between hypervisor and guest, so
it should be exposed to user space.

Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/s390/include/asm/Kbuild |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/s390/include/asm/Kbuild b/arch/s390/include/asm/Kbuild
index 63a2341..ee7937d 100644
--- a/arch/s390/include/asm/Kbuild
+++ b/arch/s390/include/asm/Kbuild
@@ -8,6 +8,7 @@ header-y += ucontext.h
 header-y += vtoc.h
 header-y += zcrypt.h
 header-y += chsc.h
+header-y += kvm_virtio.h
 
 unifdef-y += cmb.h
 unifdef-y += debug.h
-- 
1.6.0.2

Re: [PATCH 1/3] S390: take a full byte as ext_param indicator

[Marcelo Tosatti]

On Tue, Aug 24, 2010 at 03:48:50PM +0200, Alexander Graf wrote:
> Currenty the ext_param field only distinguishes between "config change" and
> "vring interrupt". We can do a lot more with it though, so let's enable a
> full byte of possible values and constants to #defines while at it.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - move defines to virtio_s390.h
> ---
>  arch/s390/include/asm/kvm_virtio.h |    6 ++++++
>  drivers/s390/kvm/kvm_virtio.c      |   19 +++++++++++++------
>  2 files changed, 19 insertions(+), 6 deletions(-)

Applied, thanks.

Re: [PATCH 1/3] S390: take a full byte as ext_param indicator

[Marcelo Tosatti]

On Tue, Aug 24, 2010 at 03:48:50PM +0200, Alexander Graf wrote:
> Currenty the ext_param field only distinguishes between "config change" and
> "vring interrupt". We can do a lot more with it though, so let's enable a
> full byte of possible values and constants to #defines while at it.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - move defines to virtio_s390.h
> ---
>  arch/s390/include/asm/kvm_virtio.h |    6 ++++++
>  drivers/s390/kvm/kvm_virtio.c      |   19 +++++++++++++------
>  2 files changed, 19 insertions(+), 6 deletions(-)

Applied, thanks.