* [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits
@ 2018-03-07 15:10 Claudio Imbrenda
2018-03-07 15:55 ` Cornelia Huck
2018-03-08 7:41 ` Christian Borntraeger
0 siblings, 2 replies; 6+ messages in thread
From: Claudio Imbrenda @ 2018-03-07 15:10 UTC (permalink / raw)
To: cohuck; +Cc: qemu-s390x, qemu-devel, borntraeger
Extend the SCLP event masks to 64 bits.
Notice that using any of the new bits results in a state that cannot be
migrated to an older version.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
---
hw/s390x/event-facility.c | 49 ++++++++++++++++++++++++++++++++-------
include/hw/s390x/event-facility.h | 2 +-
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index e04ed9f..c166e0a 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -30,7 +30,10 @@ struct SCLPEventFacility {
SysBusDevice parent_obj;
SCLPEventsBus sbus;
/* guest's receive mask */
- sccb_mask_t receive_mask;
+ union {
+ uint32_t receive_mask_pieces[2];
+ sccb_mask_t receive_mask;
+ };
/*
* when false, we keep the same broken, backwards compatible behaviour as
* before, allowing only masks of size exactly 4; when true, we implement
@@ -262,7 +265,7 @@ static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
case SCLP_SELECTIVE_READ:
copy_mask((uint8_t *)&sclp_active_selection_mask, (uint8_t *)&red->mask,
sizeof(sclp_active_selection_mask), ef->mask_length);
- sclp_active_selection_mask = be32_to_cpu(sclp_active_selection_mask);
+ sclp_active_selection_mask = be64_to_cpu(sclp_active_selection_mask);
if (!sclp_cp_receive_mask ||
(sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
sccb->h.response_code =
@@ -294,21 +297,22 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
}
/*
- * Note: We currently only support masks up to 4 byte length;
- * the remainder is filled up with zeroes. Linux uses
- * a 4 byte mask length.
+ * Note: We currently only support masks up to 8 byte length;
+ * the remainder is filled up with zeroes. Older Linux
+ * kernels use a 4 byte mask length, newer ones can use both
+ * 8 or 4 depending on what is available on the host.
*/
/* keep track of the guest's capability masks */
copy_mask((uint8_t *)&tmp_mask, WEM_CP_RECEIVE_MASK(we_mask, mask_length),
sizeof(tmp_mask), mask_length);
- ef->receive_mask = be32_to_cpu(tmp_mask);
+ ef->receive_mask = be64_to_cpu(tmp_mask);
/* return the SCLP's capability masks to the guest */
- tmp_mask = cpu_to_be32(get_host_receive_mask(ef));
+ tmp_mask = cpu_to_be64(get_host_receive_mask(ef));
copy_mask(WEM_RECEIVE_MASK(we_mask, mask_length), (uint8_t *)&tmp_mask,
mask_length, sizeof(tmp_mask));
- tmp_mask = cpu_to_be32(get_host_send_mask(ef));
+ tmp_mask = cpu_to_be64(get_host_send_mask(ef));
copy_mask(WEM_SEND_MASK(we_mask, mask_length), (uint8_t *)&tmp_mask,
mask_length, sizeof(tmp_mask));
@@ -369,6 +373,13 @@ static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
}
}
+static bool vmstate_event_facility_mask64_needed(void *opaque)
+{
+ SCLPEventFacility *ef = opaque;
+
+ return (ef->receive_mask & 0xFFFFFFFF) != 0;
+}
+
static bool vmstate_event_facility_mask_length_needed(void *opaque)
{
SCLPEventFacility *ef = opaque;
@@ -376,6 +387,21 @@ static bool vmstate_event_facility_mask_length_needed(void *opaque)
return ef->allow_all_mask_sizes;
}
+static const VMStateDescription vmstate_event_facility_mask64 = {
+ .name = "vmstate-event-facility/mask64",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .needed = vmstate_event_facility_mask64_needed,
+ .fields = (VMStateField[]) {
+#ifdef HOST_WORDS_BIGENDIAN
+ VMSTATE_UINT32(receive_mask_pieces[1], SCLPEventFacility),
+#else
+ VMSTATE_UINT32(receive_mask_pieces[0], SCLPEventFacility),
+#endif
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_event_facility_mask_length = {
.name = "vmstate-event-facility/mask_length",
.version_id = 0,
@@ -392,10 +418,15 @@ static const VMStateDescription vmstate_event_facility = {
.version_id = 0,
.minimum_version_id = 0,
.fields = (VMStateField[]) {
- VMSTATE_UINT32(receive_mask, SCLPEventFacility),
+#ifdef HOST_WORDS_BIGENDIAN
+ VMSTATE_UINT32(receive_mask_pieces[0], SCLPEventFacility),
+#else
+ VMSTATE_UINT32(receive_mask_pieces[1], SCLPEventFacility),
+#endif
VMSTATE_END_OF_LIST()
},
.subsections = (const VMStateDescription * []) {
+ &vmstate_event_facility_mask64,
&vmstate_event_facility_mask_length,
NULL
}
diff --git a/include/hw/s390x/event-facility.h b/include/hw/s390x/event-facility.h
index 0e2b761..06ba4ea 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -73,7 +73,7 @@ typedef struct WriteEventMask {
#define WEM_RECEIVE_MASK(wem, mask_len) ((wem)->masks + 2 * (mask_len))
#define WEM_SEND_MASK(wem, mask_len) ((wem)->masks + 3 * (mask_len))
-typedef uint32_t sccb_mask_t;
+typedef uint64_t sccb_mask_t;
typedef struct EventBufferHeader {
uint16_t length;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits
2018-03-07 15:10 [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits Claudio Imbrenda
@ 2018-03-07 15:55 ` Cornelia Huck
2018-03-07 16:54 ` Claudio Imbrenda
2018-03-08 7:41 ` Christian Borntraeger
1 sibling, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2018-03-07 15:55 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: qemu-s390x, qemu-devel, borntraeger
On Wed, 7 Mar 2018 16:10:34 +0100
Claudio Imbrenda <imbrenda@linux.vnet.ibm.com> wrote:
> Extend the SCLP event masks to 64 bits.
>
> Notice that using any of the new bits results in a state that cannot be
> migrated to an older version.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> ---
> hw/s390x/event-facility.c | 49 ++++++++++++++++++++++++++++++++-------
> include/hw/s390x/event-facility.h | 2 +-
> 2 files changed, 41 insertions(+), 10 deletions(-)
>
> @@ -376,6 +387,21 @@ static bool vmstate_event_facility_mask_length_needed(void *opaque)
> return ef->allow_all_mask_sizes;
> }
>
> +static const VMStateDescription vmstate_event_facility_mask64 = {
> + .name = "vmstate-event-facility/mask64",
Should that really be called 'mask64' - you're just transferring the
missing part of the mask here, aren't you?
> + .version_id = 0,
> + .minimum_version_id = 0,
> + .needed = vmstate_event_facility_mask64_needed,
> + .fields = (VMStateField[]) {
> +#ifdef HOST_WORDS_BIGENDIAN
> + VMSTATE_UINT32(receive_mask_pieces[1], SCLPEventFacility),
> +#else
> + VMSTATE_UINT32(receive_mask_pieces[0], SCLPEventFacility),
> +#endif
That #ifdef is kind of ugly, and a bit confusing, I think.
What about doing something like
/* we need to save 32 bit chunks for compatibility */
#ifdef HOST_WORDS_BIGENDIAN
#define RECV_MASK_LOWER 0
#define RECV_MASK_UPPER 1
#else /* little endian host */
#define RECV_MASK_LOWER 1
#define RECV_MASK_UPPER 0
#endif
and transfer receive_mask_pieces[RECV_MASK_UPPER] here...
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static const VMStateDescription vmstate_event_facility_mask_length = {
> .name = "vmstate-event-facility/mask_length",
> .version_id = 0,
> @@ -392,10 +418,15 @@ static const VMStateDescription vmstate_event_facility = {
> .version_id = 0,
> .minimum_version_id = 0,
> .fields = (VMStateField[]) {
> - VMSTATE_UINT32(receive_mask, SCLPEventFacility),
> +#ifdef HOST_WORDS_BIGENDIAN
> + VMSTATE_UINT32(receive_mask_pieces[0], SCLPEventFacility),
> +#else
> + VMSTATE_UINT32(receive_mask_pieces[1], SCLPEventFacility),
> +#endif
...and receive_mask_pieces[REV_MASK_LOWER] here?
(And I guess 64bit receive masks should be enough for everybody? IOW, we
don't expect a similar dance in the near future?)
> VMSTATE_END_OF_LIST()
> },
> .subsections = (const VMStateDescription * []) {
> + &vmstate_event_facility_mask64,
> &vmstate_event_facility_mask_length,
> NULL
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits
2018-03-07 15:55 ` Cornelia Huck
@ 2018-03-07 16:54 ` Claudio Imbrenda
0 siblings, 0 replies; 6+ messages in thread
From: Claudio Imbrenda @ 2018-03-07 16:54 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-s390x, qemu-devel, borntraeger
On Wed, 7 Mar 2018 16:55:48 +0100
Cornelia Huck <cohuck@redhat.com> wrote:
> On Wed, 7 Mar 2018 16:10:34 +0100
> Claudio Imbrenda <imbrenda@linux.vnet.ibm.com> wrote:
>
> > Extend the SCLP event masks to 64 bits.
> >
> > Notice that using any of the new bits results in a state that
> > cannot be migrated to an older version.
> >
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> > ---
> > hw/s390x/event-facility.c | 49
> > ++++++++++++++++++++++++++++++++-------
> > include/hw/s390x/event-facility.h | 2 +- 2 files changed, 41
> > insertions(+), 10 deletions(-)
>
> > @@ -376,6 +387,21 @@ static bool
> > vmstate_event_facility_mask_length_needed(void *opaque) return
> > ef->allow_all_mask_sizes; }
> >
> > +static const VMStateDescription vmstate_event_facility_mask64 = {
> > + .name = "vmstate-event-facility/mask64",
>
> Should that really be called 'mask64' - you're just transferring the
> missing part of the mask here, aren't you?
>
> > + .version_id = 0,
> > + .minimum_version_id = 0,
> > + .needed = vmstate_event_facility_mask64_needed,
> > + .fields = (VMStateField[]) {
> > +#ifdef HOST_WORDS_BIGENDIAN
> > + VMSTATE_UINT32(receive_mask_pieces[1], SCLPEventFacility),
> > +#else
> > + VMSTATE_UINT32(receive_mask_pieces[0], SCLPEventFacility),
> > +#endif
>
> That #ifdef is kind of ugly, and a bit confusing, I think.
>
> What about doing something like
>
> /* we need to save 32 bit chunks for compatibility */
> #ifdef HOST_WORDS_BIGENDIAN
> #define RECV_MASK_LOWER 0
> #define RECV_MASK_UPPER 1
> #else /* little endian host */
> #define RECV_MASK_LOWER 1
> #define RECV_MASK_UPPER 0
> #endif
>
> and transfer receive_mask_pieces[RECV_MASK_UPPER] here...
yes, this is better, I'll respin
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > static const VMStateDescription vmstate_event_facility_mask_length
> > = { .name = "vmstate-event-facility/mask_length",
> > .version_id = 0,
> > @@ -392,10 +418,15 @@ static const VMStateDescription
> > vmstate_event_facility = { .version_id = 0,
> > .minimum_version_id = 0,
> > .fields = (VMStateField[]) {
> > - VMSTATE_UINT32(receive_mask, SCLPEventFacility),
> > +#ifdef HOST_WORDS_BIGENDIAN
> > + VMSTATE_UINT32(receive_mask_pieces[0], SCLPEventFacility),
> > +#else
> > + VMSTATE_UINT32(receive_mask_pieces[1], SCLPEventFacility),
> > +#endif
>
> ...and receive_mask_pieces[REV_MASK_LOWER] here?
>
> (And I guess 64bit receive masks should be enough for everybody? IOW,
> we don't expect a similar dance in the near future?)
well, by then we'll need proper bitfields, so everything will need to
be rewritten anyway :)
> > VMSTATE_END_OF_LIST()
> > },
> > .subsections = (const VMStateDescription * []) {
> > + &vmstate_event_facility_mask64,
> > &vmstate_event_facility_mask_length,
> > NULL
> > }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits
2018-03-07 15:10 [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits Claudio Imbrenda
2018-03-07 15:55 ` Cornelia Huck
@ 2018-03-08 7:41 ` Christian Borntraeger
2018-03-08 10:02 ` Claudio Imbrenda
1 sibling, 1 reply; 6+ messages in thread
From: Christian Borntraeger @ 2018-03-08 7:41 UTC (permalink / raw)
To: Claudio Imbrenda, cohuck; +Cc: qemu-s390x, qemu-devel
On 03/07/2018 04:10 PM, Claudio Imbrenda wrote:
> Extend the SCLP event masks to 64 bits.
>
> Notice that using any of the new bits results in a state that cannot be
> migrated to an older version.
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> ---
> hw/s390x/event-facility.c | 49 ++++++++++++++++++++++++++++++++-------
> include/hw/s390x/event-facility.h | 2 +-
> 2 files changed, 41 insertions(+), 10 deletions(-)
>
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index e04ed9f..c166e0a 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -30,7 +30,10 @@ struct SCLPEventFacility {
> SysBusDevice parent_obj;
> SCLPEventsBus sbus;
> /* guest's receive mask */
> - sccb_mask_t receive_mask;
> + union {
> + uint32_t receive_mask_pieces[2];
> + sccb_mask_t receive_mask;
> + };
Would it work to make sccb_mask_t a union instead?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits
2018-03-08 7:41 ` Christian Borntraeger
@ 2018-03-08 10:02 ` Claudio Imbrenda
2018-03-08 10:07 ` Christian Borntraeger
0 siblings, 1 reply; 6+ messages in thread
From: Claudio Imbrenda @ 2018-03-08 10:02 UTC (permalink / raw)
To: Christian Borntraeger; +Cc: cohuck, qemu-s390x, qemu-devel
On Thu, 8 Mar 2018 08:41:47 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 03/07/2018 04:10 PM, Claudio Imbrenda wrote:
> > Extend the SCLP event masks to 64 bits.
> >
> > Notice that using any of the new bits results in a state that
> > cannot be migrated to an older version.
> >
> > Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> > ---
> > hw/s390x/event-facility.c | 49
> > ++++++++++++++++++++++++++++++++-------
> > include/hw/s390x/event-facility.h | 2 +- 2 files changed, 41
> > insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > index e04ed9f..c166e0a 100644
> > --- a/hw/s390x/event-facility.c
> > +++ b/hw/s390x/event-facility.c
> > @@ -30,7 +30,10 @@ struct SCLPEventFacility {
> > SysBusDevice parent_obj;
> > SCLPEventsBus sbus;
> > /* guest's receive mask */
> > - sccb_mask_t receive_mask;
> > + union {
> > + uint32_t receive_mask_pieces[2];
> > + sccb_mask_t receive_mask;
> > + };
>
> Would it work to make sccb_mask_t a union instead?
yes, but then a lot of other code should be changed: we use sccb_mask_t
as scalar everywhere.
also, we only need the pieces when migrating the state, never during
normal operation
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits
2018-03-08 10:02 ` Claudio Imbrenda
@ 2018-03-08 10:07 ` Christian Borntraeger
0 siblings, 0 replies; 6+ messages in thread
From: Christian Borntraeger @ 2018-03-08 10:07 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: cohuck, qemu-s390x, qemu-devel
On 03/08/2018 11:02 AM, Claudio Imbrenda wrote:
> On Thu, 8 Mar 2018 08:41:47 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>> On 03/07/2018 04:10 PM, Claudio Imbrenda wrote:
>>> Extend the SCLP event masks to 64 bits.
>>>
>>> Notice that using any of the new bits results in a state that
>>> cannot be migrated to an older version.
>>>
>>> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
>>> ---
>>> hw/s390x/event-facility.c | 49
>>> ++++++++++++++++++++++++++++++++-------
>>> include/hw/s390x/event-facility.h | 2 +- 2 files changed, 41
>>> insertions(+), 10 deletions(-)
>>>
>>> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
>>> index e04ed9f..c166e0a 100644
>>> --- a/hw/s390x/event-facility.c
>>> +++ b/hw/s390x/event-facility.c
>>> @@ -30,7 +30,10 @@ struct SCLPEventFacility {
>>> SysBusDevice parent_obj;
>>> SCLPEventsBus sbus;
>>> /* guest's receive mask */
>>> - sccb_mask_t receive_mask;
>>> + union {
>>> + uint32_t receive_mask_pieces[2];
>>> + sccb_mask_t receive_mask;
>>> + };
>>
>> Would it work to make sccb_mask_t a union instead?
>
> yes, but then a lot of other code should be changed: we use sccb_mask_t
> as scalar everywhere.
>
> also, we only need the pieces when migrating the state, never during
> normal operation
ok, makes sense.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-08 10:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-07 15:10 [Qemu-devel] [PATCH v5 1/1] s390x/sclp: extend SCLP event masks to 64 bits Claudio Imbrenda
2018-03-07 15:55 ` Cornelia Huck
2018-03-07 16:54 ` Claudio Imbrenda
2018-03-08 7:41 ` Christian Borntraeger
2018-03-08 10:02 ` Claudio Imbrenda
2018-03-08 10:07 ` Christian Borntraeger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.