Re: [Qemu-devel] [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 
Acked-by: Christian Borntraeger 
Reviewed-by: Dong Jia Shi 

---
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]) {

Re: [Qemu-devel] [PATCH 2/3] s390x/css: advertise unrestricted cssids

[Shalini Chellathurai Saroja]

On 12/04/2017 04:07 PM, Halil Pasic wrote:


On 12/04/2017 12:15 PM, Cornelia Huck wrote:

On Fri,  1 Dec 2017 15:31:35 +0100
Halil Pasic  wrote:


Let us advertise the changes introduced by "s390x/css: unrestrict cssids"
to the management software (so it can tell are cssids unrestricted or
restricted).

Signed-off-by: Halil Pasic 
---

Boris says having the property on the virtual-css-bridge is good form
Libvirt PoV. @Shalini: could you verify that things work out fine
(provided we get at least a preliminary blessing from Connie).

Consider squashing into "s390x/css: unrestrict cssids".
---
  hw/s390x/css-bridge.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
index c4a9735d71..c7e8998680 100644
--- a/hw/s390x/css-bridge.c
+++ b/hw/s390x/css-bridge.c
@@ -123,6 +123,11 @@ static Property virtual_css_bridge_properties[] = {
  DEFINE_PROP_END_OF_LIST(),
  };
  
+static bool prop_get_true(Object *obj, Error **errp)

+{
+return true;
+}
+
  static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
  {
  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
@@ -131,6 +136,12 @@ static void virtual_css_bridge_class_init(ObjectClass 
*klass, void *data)
  hc->unplug = ccw_device_unplug;
  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
  dc->props = virtual_css_bridge_properties;
+object_class_property_add_bool(klass, "cssid-unrestricted",
+   prop_get_true, NULL, NULL);
+object_class_property_set_description(klass, "cssid-unrestricted",
+"A css device can use any  cssid, regardless whether virtual"

extra space -^


Nod.


+" or not (read only, always true)",

Do we need "." here ^ ?


+NULL);
  }
  
  static const TypeInfo virtual_css_bridge_info = {

Looks reasonable. If this works as expected, I'll squash it into the
previous patch.


I've just asked Shalini to verify the libvirt perspective.


Verified. The patch works as expected.



Supposed we verify this works as expected, I read I don't have to spin
a v2 and you are going to fix the issues found yourself. Right?