Re: [qemu-s390x] [RFC PATCH 1/1] s390x/css: unresrict cssids

[Shalini Chellathurai Saroja]

On 11/21/2017 12:18 PM, Halil Pasic wrote:
> The default css 0xFE is currently restricted to virtual subchannel
> devices. The hope when the decision was made was, that non-virtual
> subchannel devices will come around when guest can exploit multiple
> channel subsystems. Since the guests generally don't do, the pain
> of the partitioned (cssid) namespace outweighs the gain.
>
> Let us remove the corresponding restrictions (virtual devices
> can be put only in 0xFE and non-virtual devices in any css except
> the 0xFE), and inform management software property on all ccw
> devices.
>
> The adverse effect on migration should not be too severe as
> vfio-ccw devices are not live-migratable yet, and for virtual
> devices using the extra freedom would only make sense with
> the aforementioned guest support in place.
>
> Signed-off-by: Halil Pasic <address@hidden>
> Acked-by: Christian Borntraeger <address@hidden>
> Reviewed-by: Dong Jia Shi <address@hidden>
>
> ---
> Hi!
>
> About indicating this at the ccw devices instead of, e.g. as a machine
> property (or otherwise globally), was requested by our libvirt guys. I
> have no strong opinion regarding in this matter.
>
> This patch is intended as a discussion starter. I would at least like to
> get a Tested-by by Shalini before promoting it to non-RFC (provided the
> discussion goes well).
Tested the patch in libvirt. It works as expected (allows to specify 
cssid of vfio devices as 0xfe).
Thank you.
> TODOs:
> * Consider adding a description for the new property.
> * Consider renaming VIRTUAL_CSSID.
> * Consider changing the bus-id generation scheme (when
> devno is not specified by the user). his patch keep the current scheme in
> place: they won't go into the default css (but slots are filled up
> starting at cssid 0). This is theoretically good for migration
> compatibility same command line same addresses.  Practically since there
> is no migratable non-virtual ccw device, we should consider using the
> same bus-id generation scheme for virtual and non-virtual devices.
>
> ---
>   hw/s390x/ccw-device.c | 6 ++++++
>   hw/s390x/css.c        | 9 ---------
>   2 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
> index f9bfa154d6..2167ccea5d 100644
> --- a/hw/s390x/ccw-device.c
> +++ b/hw/s390x/ccw-device.c
> @@ -40,6 +40,10 @@ static Property ccw_device_properties[] = {
>       DEFINE_PROP_END_OF_LIST(),
>   };
>
> +static bool prop_get_true(Object *obj, Error **errp)
> +{
> +    return true;
> +}
>   static void ccw_device_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -48,6 +52,8 @@ static void ccw_device_class_init(ObjectClass *klass, void 
> *data)
>       k->realize = ccw_device_realize;
>       k->refill_ids = ccw_device_refill_ids;
>       dc->props = ccw_device_properties;
> +    object_class_property_add_bool(klass, "cssid-unrestricted",
> +                                   prop_get_true, NULL, NULL);
>   }
>
>   const VMStateDescription vmstate_ccw_dev = {
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index f6b5c807cd..957cb9ec90 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -2377,15 +2377,6 @@ SubchDev *css_create_sch(CssDevId bus_id, bool 
> is_virtual, bool squash_mcss,
>       SubchDev *sch;
>
>       if (bus_id.valid) {
> -        if (is_virtual != (bus_id.cssid == VIRTUAL_CSSID)) {
> -            error_setg(errp, "cssid %hhx not valid for %s devices",
> -                       bus_id.cssid,
> -                       (is_virtual ? "virtual" : "non-virtual"));
> -            return NULL;
> -        }
> -    }
> -
> -    if (bus_id.valid) {
>           if (squash_mcss) {
>               bus_id.cssid = channel_subsys.default_cssid;
>           } else if (!channel_subsys.css[bus_id.cssid]) {