* Re: [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false
2017-10-04 13:53 [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false Thomas Huth
@ 2017-10-04 14:00 ` Claudio Imbrenda
2017-10-04 14:22 ` Farhan Ali
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Claudio Imbrenda @ 2017-10-04 14:00 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, Cornelia Huck, Christian Borntraeger, Dong Jia Shi,
Eric Farman, Farhan Ali, Fei Li, Halil Pasic, Janosch Frank,
Jason J Herne, Jing Liu, Pierre Morel, QingFeng Hao,
Xiao Feng Ren, Yang Chen, Yi Min Zhao
On Wed, 4 Oct 2017 15:53:19 +0200
Thomas Huth <thuth@redhat.com> wrote:
> The "sclp" device is just an internal device that can not be
> instantiated by the users. If they try to use it, they only get a
> simple error message:
>
> $ qemu-system-s390x -nographic -device sclp
> qemu-system-s390x: Option '-device s390-sclp-event-facility' cannot be
> handled by this machine
>
> Since sclp_init() tries to create a TYPE_SCLP_EVENT_FACILITY which is
> a non-pluggable sysbus device, there is really no way that the "sclp"
> device can be used by the user, so let's set the user_creatable =
> false accordingly.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/s390x/sclp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 30aefbf..9be0cb8 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -606,6 +606,11 @@ static void sclp_class_init(ObjectClass *oc,
> void *data) dc->realize = sclp_realize;
> dc->hotpluggable = false;
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> + /*
> + * Reason: Creates TYPE_SCLP_EVENT_FACILITY in sclp_init
> + * which is a non-pluggable sysbus device
> + */
> + dc->user_creatable = false;
>
> sc->read_SCP_info = read_SCP_info;
> sc->read_storage_element0_info = read_storage_element0_info;
makes sense
Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false
2017-10-04 13:53 [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false Thomas Huth
2017-10-04 14:00 ` Claudio Imbrenda
@ 2017-10-04 14:22 ` Farhan Ali
2017-10-04 14:56 ` Halil Pasic
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Farhan Ali @ 2017-10-04 14:22 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Cornelia Huck, Christian Borntraeger
Cc: Claudio Imbrenda, Dong Jia Shi, Eric Farman, Fei Li, Halil Pasic,
Janosch Frank, Jason J Herne, Jing Liu, Pierre Morel,
QingFeng Hao, Xiao Feng Ren, Yang Chen, Yi Min Zhao
On 10/04/2017 09:53 AM, Thomas Huth wrote:
> The "sclp" device is just an internal device that can not be instantiated
> by the users. If they try to use it, they only get a simple error message:
>
> $ qemu-system-s390x -nographic -device sclp
> qemu-system-s390x: Option '-device s390-sclp-event-facility' cannot be
> handled by this machine
>
> Since sclp_init() tries to create a TYPE_SCLP_EVENT_FACILITY which is
> a non-pluggable sysbus device, there is really no way that the "sclp"
> device can be used by the user, so let's set the user_creatable = false
> accordingly.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/s390x/sclp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 30aefbf..9be0cb8 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -606,6 +606,11 @@ static void sclp_class_init(ObjectClass *oc, void *data)
> dc->realize = sclp_realize;
> dc->hotpluggable = false;
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> + /*
> + * Reason: Creates TYPE_SCLP_EVENT_FACILITY in sclp_init
> + * which is a non-pluggable sysbus device
> + */
> + dc->user_creatable = false;
>
> sc->read_SCP_info = read_SCP_info;
> sc->read_storage_element0_info = read_storage_element0_info;
>
Reviewed-by: Farhan Ali <alifm@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false
2017-10-04 13:53 [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false Thomas Huth
2017-10-04 14:00 ` Claudio Imbrenda
2017-10-04 14:22 ` Farhan Ali
@ 2017-10-04 14:56 ` Halil Pasic
2017-10-04 15:18 ` Pierre Morel
2017-10-05 8:04 ` Cornelia Huck
4 siblings, 0 replies; 9+ messages in thread
From: Halil Pasic @ 2017-10-04 14:56 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Cornelia Huck, Christian Borntraeger
Cc: Eric Farman, Janosch Frank, Yi Min Zhao, Jason J Herne,
Pierre Morel, Farhan Ali, Jing Liu, Claudio Imbrenda,
QingFeng Hao, Xiao Feng Ren, Fei Li, Dong Jia Shi, Yang Chen
On 10/04/2017 03:53 PM, Thomas Huth wrote:
> The "sclp" device is just an internal device that can not be instantiated
> by the users. If they try to use it, they only get a simple error message:
>
> $ qemu-system-s390x -nographic -device sclp
> qemu-system-s390x: Option '-device s390-sclp-event-facility' cannot be
> handled by this machine
>
> Since sclp_init() tries to create a TYPE_SCLP_EVENT_FACILITY which is
> a non-pluggable sysbus device, there is really no way that the "sclp"
> device can be used by the user, so let's set the user_creatable = false
> accordingly.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
>
While I'm not really familiar with the sclp code, I do understand the
measure (user_creatable = false), but I'm not sure I understand the
explanation. So it's:
Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---
> hw/s390x/sclp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 30aefbf..9be0cb8 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -606,6 +606,11 @@ static void sclp_class_init(ObjectClass *oc, void *data)
> dc->realize = sclp_realize;
> dc->hotpluggable = false;
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> + /*
> + * Reason: Creates TYPE_SCLP_EVENT_FACILITY in sclp_init
> + * which is a non-pluggable sysbus device
> + */
> + dc->user_creatable = false;
>
> sc->read_SCP_info = read_SCP_info;
> sc->read_storage_element0_info = read_storage_element0_info;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false
2017-10-04 13:53 [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false Thomas Huth
` (2 preceding siblings ...)
2017-10-04 14:56 ` Halil Pasic
@ 2017-10-04 15:18 ` Pierre Morel
2017-10-04 15:42 ` Cornelia Huck
2017-10-04 17:06 ` Thomas Huth
2017-10-05 8:04 ` Cornelia Huck
4 siblings, 2 replies; 9+ messages in thread
From: Pierre Morel @ 2017-10-04 15:18 UTC (permalink / raw)
To: qemu-devel
On 04/10/2017 15:53, Thomas Huth wrote:
> The "sclp" device is just an internal device that can not be instantiated
> by the users. If they try to use it, they only get a simple error message:
>
> $ qemu-system-s390x -nographic -device sclp
> qemu-system-s390x: Option '-device s390-sclp-event-facility' cannot be
> handled by this machine
>
> Since sclp_init() tries to create a TYPE_SCLP_EVENT_FACILITY which is
> a non-pluggable sysbus device, there is really no way that the "sclp"
> device can be used by the user, so let's set the user_creatable = false
> accordingly.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/s390x/sclp.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 30aefbf..9be0cb8 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -606,6 +606,11 @@ static void sclp_class_init(ObjectClass *oc, void *data)
> dc->realize = sclp_realize;
> dc->hotpluggable = false;
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> + /*
> + * Reason: Creates TYPE_SCLP_EVENT_FACILITY in sclp_init
> + * which is a non-pluggable sysbus device
> + */
> + dc->user_creatable = false;
>
> sc->read_SCP_info = read_SCP_info;
> sc->read_storage_element0_info = read_storage_element0_info;
>
I must miss something.
Why is the sclp device not a SYS_BUS_DEVICE ?
The problem seems to come from the heterogeneity of the sclp and sclp
generated devices.
--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false
2017-10-04 15:18 ` Pierre Morel
@ 2017-10-04 15:42 ` Cornelia Huck
2017-10-04 17:06 ` Thomas Huth
1 sibling, 0 replies; 9+ messages in thread
From: Cornelia Huck @ 2017-10-04 15:42 UTC (permalink / raw)
To: Pierre Morel; +Cc: qemu-devel
On Wed, 4 Oct 2017 17:18:57 +0200
Pierre Morel <pmorel@linux.vnet.ibm.com> wrote:
> On 04/10/2017 15:53, Thomas Huth wrote:
> > The "sclp" device is just an internal device that can not be instantiated
> > by the users. If they try to use it, they only get a simple error message:
> >
> > $ qemu-system-s390x -nographic -device sclp
> > qemu-system-s390x: Option '-device s390-sclp-event-facility' cannot be
> > handled by this machine
> >
> > Since sclp_init() tries to create a TYPE_SCLP_EVENT_FACILITY which is
> > a non-pluggable sysbus device, there is really no way that the "sclp"
> > device can be used by the user, so let's set the user_creatable = false
> > accordingly.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> > hw/s390x/sclp.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index 30aefbf..9be0cb8 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -606,6 +606,11 @@ static void sclp_class_init(ObjectClass *oc, void *data)
> > dc->realize = sclp_realize;
> > dc->hotpluggable = false;
> > set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > + /*
> > + * Reason: Creates TYPE_SCLP_EVENT_FACILITY in sclp_init
> > + * which is a non-pluggable sysbus device
> > + */
> > + dc->user_creatable = false;
> >
> > sc->read_SCP_info = read_SCP_info;
> > sc->read_storage_element0_info = read_storage_element0_info;
> >
>
> I must miss something.
> Why is the sclp device not a SYS_BUS_DEVICE ?
> The problem seems to come from the heterogeneity of the sclp and sclp
> generated devices.
>
I think the problem is 'not pluggable' rather than whether it is or
is not a sysbus device, no?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false
2017-10-04 15:18 ` Pierre Morel
2017-10-04 15:42 ` Cornelia Huck
@ 2017-10-04 17:06 ` Thomas Huth
2017-10-05 8:14 ` Cornelia Huck
1 sibling, 1 reply; 9+ messages in thread
From: Thomas Huth @ 2017-10-04 17:06 UTC (permalink / raw)
To: Pierre Morel, qemu-devel
On 04.10.2017 17:18, Pierre Morel wrote:
> On 04/10/2017 15:53, Thomas Huth wrote:
>> The "sclp" device is just an internal device that can not be instantiated
>> by the users. If they try to use it, they only get a simple error
>> message:
>>
>> $ qemu-system-s390x -nographic -device sclp
>> qemu-system-s390x: Option '-device s390-sclp-event-facility' cannot be
>> handled by this machine
>>
>> Since sclp_init() tries to create a TYPE_SCLP_EVENT_FACILITY which is
>> a non-pluggable sysbus device, there is really no way that the "sclp"
>> device can be used by the user, so let's set the user_creatable = false
>> accordingly.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/s390x/sclp.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 30aefbf..9be0cb8 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -606,6 +606,11 @@ static void sclp_class_init(ObjectClass *oc, void
>> *data)
>> dc->realize = sclp_realize;
>> dc->hotpluggable = false;
>> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>> + /*
>> + * Reason: Creates TYPE_SCLP_EVENT_FACILITY in sclp_init
>> + * which is a non-pluggable sysbus device
>> + */
>> + dc->user_creatable = false;
>>
>> sc->read_SCP_info = read_SCP_info;
>> sc->read_storage_element0_info = read_storage_element0_info;
>>
>
> I must miss something.
> Why is the sclp device not a SYS_BUS_DEVICE ?
> The problem seems to come from the heterogeneity of the sclp and sclp
> generated devices.
I wonder whether it should rather be the other way round: Why is
TYPE_SCLP_EVENT_FACILITY a sysbus device? Sysbus is an abstraction for
simple on-board devices, while TYPE_SCLP_EVENT_FACILITY seems rather to
be a pseudo-device... so it should rather simply be of TYPE_DEVICE
instead? Or do I miss something?
Thomas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false
2017-10-04 17:06 ` Thomas Huth
@ 2017-10-05 8:14 ` Cornelia Huck
0 siblings, 0 replies; 9+ messages in thread
From: Cornelia Huck @ 2017-10-05 8:14 UTC (permalink / raw)
To: Thomas Huth; +Cc: Pierre Morel, qemu-devel
On Wed, 4 Oct 2017 19:06:57 +0200
Thomas Huth <thuth@redhat.com> wrote:
> On 04.10.2017 17:18, Pierre Morel wrote:
> > On 04/10/2017 15:53, Thomas Huth wrote:
> >> The "sclp" device is just an internal device that can not be instantiated
> >> by the users. If they try to use it, they only get a simple error
> >> message:
> >>
> >> $ qemu-system-s390x -nographic -device sclp
> >> qemu-system-s390x: Option '-device s390-sclp-event-facility' cannot be
> >> handled by this machine
> >>
> >> Since sclp_init() tries to create a TYPE_SCLP_EVENT_FACILITY which is
> >> a non-pluggable sysbus device, there is really no way that the "sclp"
> >> device can be used by the user, so let's set the user_creatable = false
> >> accordingly.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >> hw/s390x/sclp.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> >> index 30aefbf..9be0cb8 100644
> >> --- a/hw/s390x/sclp.c
> >> +++ b/hw/s390x/sclp.c
> >> @@ -606,6 +606,11 @@ static void sclp_class_init(ObjectClass *oc, void
> >> *data)
> >> dc->realize = sclp_realize;
> >> dc->hotpluggable = false;
> >> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> >> + /*
> >> + * Reason: Creates TYPE_SCLP_EVENT_FACILITY in sclp_init
> >> + * which is a non-pluggable sysbus device
> >> + */
> >> + dc->user_creatable = false;
> >>
> >> sc->read_SCP_info = read_SCP_info;
> >> sc->read_storage_element0_info = read_storage_element0_info;
> >>
> >
> > I must miss something.
> > Why is the sclp device not a SYS_BUS_DEVICE ?
> > The problem seems to come from the heterogeneity of the sclp and sclp
> > generated devices.
>
> I wonder whether it should rather be the other way round: Why is
> TYPE_SCLP_EVENT_FACILITY a sysbus device? Sysbus is an abstraction for
> simple on-board devices, while TYPE_SCLP_EVENT_FACILITY seems rather to
> be a pseudo-device... so it should rather simply be of TYPE_DEVICE
> instead? Or do I miss something?
We do have a number of devices that are sysbus because you could not
have bus-less devices back then. We might want to convert them to a
simple TYPE_DEVICE, but this needs double-checking regarding resets and
other things and I'd rather not spend that much time on that right now.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false
2017-10-04 13:53 [Qemu-devel] [PATCH] hw/s390x/sclp: Mark the sclp device with user_creatable = false Thomas Huth
` (3 preceding siblings ...)
2017-10-04 15:18 ` Pierre Morel
@ 2017-10-05 8:04 ` Cornelia Huck
4 siblings, 0 replies; 9+ messages in thread
From: Cornelia Huck @ 2017-10-05 8:04 UTC (permalink / raw)
To: Thomas Huth
Cc: qemu-devel, Christian Borntraeger, Claudio Imbrenda, Dong Jia Shi,
Eric Farman, Farhan Ali, Fei Li, Halil Pasic, Janosch Frank,
Jason J Herne, Jing Liu, Pierre Morel, QingFeng Hao,
Xiao Feng Ren, Yang Chen, Yi Min Zhao
On Wed, 4 Oct 2017 15:53:19 +0200
Thomas Huth <thuth@redhat.com> wrote:
> The "sclp" device is just an internal device that can not be instantiated
> by the users. If they try to use it, they only get a simple error message:
>
> $ qemu-system-s390x -nographic -device sclp
> qemu-system-s390x: Option '-device s390-sclp-event-facility' cannot be
> handled by this machine
>
> Since sclp_init() tries to create a TYPE_SCLP_EVENT_FACILITY which is
> a non-pluggable sysbus device, there is really no way that the "sclp"
> device can be used by the user, so let's set the user_creatable = false
> accordingly.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/s390x/sclp.c | 5 +++++
> 1 file changed, 5 insertions(+)
Thanks, applied.
^ permalink raw reply [flat|nested] 9+ messages in thread