| Message ID | 20220606203614.110928-9-mjrosato@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x/pci: zPCI interpretation support | expand |
On 6/6/22 22:36, Matthew Rosato wrote: > The zpcii-disable machine property can be used to force-disable the use > of zPCI interpretation facilities for a VM. By default, this setting > will be off for machine 7.1 and newer. > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > --- > hw/s390x/s390-pci-kvm.c | 4 +++- > hw/s390x/s390-virtio-ccw.c | 24 ++++++++++++++++++++++++ > include/hw/s390x/s390-virtio-ccw.h | 1 + > qemu-options.hx | 8 +++++++- > util/qemu-config.c | 4 ++++ > 5 files changed, 39 insertions(+), 2 deletions(-) > > diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c > index 9134fe185f..5eb7fd12e2 100644 > --- a/hw/s390x/s390-pci-kvm.c > +++ b/hw/s390x/s390-pci-kvm.c > @@ -22,7 +22,9 @@ > > bool s390_pci_kvm_interp_allowed(void) > { > - return kvm_s390_get_zpci_op() && !s390_is_pv(); > + return (kvm_s390_get_zpci_op() && !s390_is_pv() && > + !object_property_get_bool(OBJECT(qdev_get_machine()), > + "zpcii-disable", NULL)); > } Isn't it a duplication of machine_get_zpcii_disable? Wouldn't it better go to hw/s390x/kvm/ ? There get the MachineState *ms = MACHINE(qdev_get_machine()) and return the ms->zpcii_disable ? > > int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib, bool assist) > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index cc3097bfee..70229b102b 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -645,6 +645,21 @@ static inline void machine_set_dea_key_wrap(Object *obj, bool value, > ms->dea_key_wrap = value; > } > > +static inline bool machine_get_zpcii_disable(Object *obj, Error **errp) > +{ > + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); > + > + return ms->zpcii_disable; > +} > + > +static inline void machine_set_zpcii_disable(Object *obj, bool value, > + Error **errp) > +{ > + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); > + > + ms->zpcii_disable = value; > +} > + > static S390CcwMachineClass *current_mc; > > /* > @@ -740,6 +755,13 @@ static inline void s390_machine_initfn(Object *obj) > "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted" > " to upper case) to pass to machine loader, boot manager," > " and guest kernel"); > + > + object_property_add_bool(obj, "zpcii-disable", > + machine_get_zpcii_disable, > + machine_set_zpcii_disable); > + object_property_set_description(obj, "zpcii-disable", > + "disable zPCI interpretation facilties"); > + object_property_set_bool(obj, "zpcii-disable", false, NULL); > } > > static const TypeInfo ccw_machine_info = { > @@ -804,9 +826,11 @@ DEFINE_CCW_MACHINE(7_1, "7.1", true); > static void ccw_machine_7_0_instance_options(MachineState *machine) > { > static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V7_0 }; > + S390CcwMachineState *ms = S390_CCW_MACHINE(machine); > > ccw_machine_7_1_instance_options(machine); > s390_set_qemu_cpu_model(0x8561, 15, 1, qemu_cpu_feat); > + ms->zpcii_disable = true; > } > > static void ccw_machine_7_0_class_options(MachineClass *mc) > diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h > index 3331990e02..8a0090a071 100644 > --- a/include/hw/s390x/s390-virtio-ccw.h > +++ b/include/hw/s390x/s390-virtio-ccw.h > @@ -27,6 +27,7 @@ struct S390CcwMachineState { > bool aes_key_wrap; > bool dea_key_wrap; > bool pv; > + bool zpcii_disable; > uint8_t loadparm[8]; > }; > > diff --git a/qemu-options.hx b/qemu-options.hx > index 60cf188da4..fafe335b4a 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -36,7 +36,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > " nvdimm=on|off controls NVDIMM support (default=off)\n" > " memory-encryption=@var{} memory encryption object to use (default=none)\n" > " hmat=on|off controls ACPI HMAT support (default=off)\n" > - " memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n", > + " memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n" > + " zpcii-disable=on|off disables zPCI interpretation facilities (default=off)\n", > QEMU_ARCH_ALL) > SRST > ``-machine [type=]name[,prop=value[,...]]`` > @@ -124,6 +125,11 @@ SRST > -object memory-backend-ram,id=pc.ram,size=512M,x-use-canonical-path-for-ramblock-id=off > -machine memory-backend=pc.ram > -m 512M > + > + ``zpcii-disable=on|off`` > + Disables zPCI interpretation facilties on s390-ccw hosts. s/facilties/facility/ > + This feature can be used to disable hardware virtual assists > + related to zPCI devices. The default is off. > ERST > > DEF("M", HAS_ARG, QEMU_OPTION_M, > diff --git a/util/qemu-config.c b/util/qemu-config.c > index 433488aa56..5325f6bf80 100644 > --- a/util/qemu-config.c > +++ b/util/qemu-config.c > @@ -236,6 +236,10 @@ static QemuOptsList machine_opts = { > .help = "Up to 8 chars in set of [A-Za-z0-9. ](lower case chars" > " converted to upper case) to pass to machine" > " loader, boot manager, and guest kernel", > + },{ > + .name = "zpcii-disable", > + .type = QEMU_OPT_BOOL, > + .help = "disable zPCI interpretation facilities", > }, > { /* End of list */ } > } >
On 6/22/22 4:50 AM, Pierre Morel wrote: > > > On 6/6/22 22:36, Matthew Rosato wrote: >> The zpcii-disable machine property can be used to force-disable the use >> of zPCI interpretation facilities for a VM. By default, this setting >> will be off for machine 7.1 and newer. >> >> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >> --- >> hw/s390x/s390-pci-kvm.c | 4 +++- >> hw/s390x/s390-virtio-ccw.c | 24 ++++++++++++++++++++++++ >> include/hw/s390x/s390-virtio-ccw.h | 1 + >> qemu-options.hx | 8 +++++++- >> util/qemu-config.c | 4 ++++ >> 5 files changed, 39 insertions(+), 2 deletions(-) >> >> diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c >> index 9134fe185f..5eb7fd12e2 100644 >> --- a/hw/s390x/s390-pci-kvm.c >> +++ b/hw/s390x/s390-pci-kvm.c >> @@ -22,7 +22,9 @@ >> bool s390_pci_kvm_interp_allowed(void) >> { >> - return kvm_s390_get_zpci_op() && !s390_is_pv(); >> + return (kvm_s390_get_zpci_op() && !s390_is_pv() && >> + !object_property_get_bool(OBJECT(qdev_get_machine()), >> + "zpcii-disable", NULL)); >> } > > Isn't it a duplication of machine_get_zpcii_disable? > No, this will actually trigger machine_get_zpcii_disable -- it was setup as the 'getter' routine in s390_machine_initfn() -- see below: > Wouldn't it better go to hw/s390x/kvm/ ? > > There get the MachineState *ms = MACHINE(qdev_get_machine()) and return > the ms->zpcii_disable > > ? > >> int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib, >> bool assist) >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index cc3097bfee..70229b102b 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -645,6 +645,21 @@ static inline void >> machine_set_dea_key_wrap(Object *obj, bool value, >> ms->dea_key_wrap = value; >> } >> +static inline bool machine_get_zpcii_disable(Object *obj, Error **errp) >> +{ >> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); >> + >> + return ms->zpcii_disable; >> +} >> + >> +static inline void machine_set_zpcii_disable(Object *obj, bool value, >> + Error **errp) >> +{ >> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); >> + >> + ms->zpcii_disable = value; >> +} >> + >> static S390CcwMachineClass *current_mc; >> /* >> @@ -740,6 +755,13 @@ static inline void s390_machine_initfn(Object *obj) >> "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars >> converted" >> " to upper case) to pass to machine loader, boot manager," >> " and guest kernel"); >> + >> + object_property_add_bool(obj, "zpcii-disable", >> + machine_get_zpcii_disable, ^^ Here.
On 6/22/22 17:20, Matthew Rosato wrote: > On 6/22/22 4:50 AM, Pierre Morel wrote: >> >> >> On 6/6/22 22:36, Matthew Rosato wrote: >>> The zpcii-disable machine property can be used to force-disable the use >>> of zPCI interpretation facilities for a VM. By default, this setting >>> will be off for machine 7.1 and newer. >>> >>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >>> --- >>> hw/s390x/s390-pci-kvm.c | 4 +++- >>> hw/s390x/s390-virtio-ccw.c | 24 ++++++++++++++++++++++++ >>> include/hw/s390x/s390-virtio-ccw.h | 1 + >>> qemu-options.hx | 8 +++++++- >>> util/qemu-config.c | 4 ++++ >>> 5 files changed, 39 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c >>> index 9134fe185f..5eb7fd12e2 100644 >>> --- a/hw/s390x/s390-pci-kvm.c >>> +++ b/hw/s390x/s390-pci-kvm.c >>> @@ -22,7 +22,9 @@ >>> bool s390_pci_kvm_interp_allowed(void) >>> { >>> - return kvm_s390_get_zpci_op() && !s390_is_pv(); >>> + return (kvm_s390_get_zpci_op() && !s390_is_pv() && >>> + !object_property_get_bool(OBJECT(qdev_get_machine()), >>> + "zpcii-disable", NULL)); >>> } >> >> Isn't it a duplication of machine_get_zpcii_disable? >> > > No, this will actually trigger machine_get_zpcii_disable -- it was setup > as the 'getter' routine in s390_machine_initfn() -- see below: OK, I did not explain myself correctly: I was curious why we do not use directly ms->zpci_disabled and use the getter. Does not mean it is false. Far from. > >> Wouldn't it better go to hw/s390x/kvm/ ? >> >> There get the MachineState *ms = MACHINE(qdev_get_machine()) and >> return the ms->zpcii_disable >> >> ? >> >>> int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib, >>> bool assist) >>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>> index cc3097bfee..70229b102b 100644 >>> --- a/hw/s390x/s390-virtio-ccw.c >>> +++ b/hw/s390x/s390-virtio-ccw.c >>> @@ -645,6 +645,21 @@ static inline void >>> machine_set_dea_key_wrap(Object *obj, bool value, >>> ms->dea_key_wrap = value; >>> } >>> +static inline bool machine_get_zpcii_disable(Object *obj, Error **errp) >>> +{ >>> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); >>> + >>> + return ms->zpcii_disable; >>> +} >>> + >>> +static inline void machine_set_zpcii_disable(Object *obj, bool value, >>> + Error **errp) >>> +{ >>> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); >>> + >>> + ms->zpcii_disable = value; >>> +} >>> + >>> static S390CcwMachineClass *current_mc; >>> /* >>> @@ -740,6 +755,13 @@ static inline void s390_machine_initfn(Object *obj) >>> "Up to 8 chars in set of [A-Za-z0-9. ] (lower case >>> chars converted" >>> " to upper case) to pass to machine loader, boot manager," >>> " and guest kernel"); >>> + >>> + object_property_add_bool(obj, "zpcii-disable", >>> + machine_get_zpcii_disable, > > ^^ Here.
On 6/23/22 9:50 AM, Pierre Morel wrote: > > > On 6/22/22 17:20, Matthew Rosato wrote: >> On 6/22/22 4:50 AM, Pierre Morel wrote: >>> >>> >>> On 6/6/22 22:36, Matthew Rosato wrote: >>>> The zpcii-disable machine property can be used to force-disable the use >>>> of zPCI interpretation facilities for a VM. By default, this setting >>>> will be off for machine 7.1 and newer. >>>> >>>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >>>> --- >>>> hw/s390x/s390-pci-kvm.c | 4 +++- >>>> hw/s390x/s390-virtio-ccw.c | 24 ++++++++++++++++++++++++ >>>> include/hw/s390x/s390-virtio-ccw.h | 1 + >>>> qemu-options.hx | 8 +++++++- >>>> util/qemu-config.c | 4 ++++ >>>> 5 files changed, 39 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c >>>> index 9134fe185f..5eb7fd12e2 100644 >>>> --- a/hw/s390x/s390-pci-kvm.c >>>> +++ b/hw/s390x/s390-pci-kvm.c >>>> @@ -22,7 +22,9 @@ >>>> bool s390_pci_kvm_interp_allowed(void) >>>> { >>>> - return kvm_s390_get_zpci_op() && !s390_is_pv(); >>>> + return (kvm_s390_get_zpci_op() && !s390_is_pv() && >>>> + !object_property_get_bool(OBJECT(qdev_get_machine()), >>>> + "zpcii-disable", NULL)); >>>> } >>> >>> Isn't it a duplication of machine_get_zpcii_disable? >>> >> >> No, this will actually trigger machine_get_zpcii_disable -- it was >> setup as the 'getter' routine in s390_machine_initfn() -- see below: > > OK, I did not explain myself correctly: > I was curious why we do not use directly ms->zpci_disabled and use the > getter. > To do so, we'd have to either call machine_get_zpcii_disable directly from here or duplicate the work machine_get_zpcii_disable does by casting the machine to S390CcwMachineState so we could look at ms->zpcii_disabled. We can't call machine_get_zpcii_disable directly as-is, it's a static routine in s390-virtio-ccw.c -- making a 'getter' routine public seems wrong, so we are left with recreating the cast and looking at ms->zpcii_disabled here; but as far as I can figure the point is to have a unified interface for querying a machine property value via object_property_get_*(). Why wouldn't we use that interface? FWIW, I modeled this after the way we today handle aes-key-wrap in target/s390x/kvm/kvm.c and loadparm in hw/s390x/ipl.c (albeit we use object_property_get_str for the latter since it's a different property type). > Does not mean it is false. Far from. > >> >>> Wouldn't it better go to hw/s390x/kvm/ ? >>> >>> There get the MachineState *ms = MACHINE(qdev_get_machine()) and >>> return the ms->zpcii_disable >>> >>> ? >>> >>>> int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib, >>>> bool assist) >>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>>> index cc3097bfee..70229b102b 100644 >>>> --- a/hw/s390x/s390-virtio-ccw.c >>>> +++ b/hw/s390x/s390-virtio-ccw.c >>>> @@ -645,6 +645,21 @@ static inline void >>>> machine_set_dea_key_wrap(Object *obj, bool value, >>>> ms->dea_key_wrap = value; >>>> } >>>> +static inline bool machine_get_zpcii_disable(Object *obj, Error >>>> **errp) >>>> +{ >>>> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); >>>> + >>>> + return ms->zpcii_disable; >>>> +} >>>> + >>>> +static inline void machine_set_zpcii_disable(Object *obj, bool value, >>>> + Error **errp) >>>> +{ >>>> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); >>>> + >>>> + ms->zpcii_disable = value; >>>> +} >>>> + >>>> static S390CcwMachineClass *current_mc; >>>> /* >>>> @@ -740,6 +755,13 @@ static inline void s390_machine_initfn(Object >>>> *obj) >>>> "Up to 8 chars in set of [A-Za-z0-9. ] (lower case >>>> chars converted" >>>> " to upper case) to pass to machine loader, boot >>>> manager," >>>> " and guest kernel"); >>>> + >>>> + object_property_add_bool(obj, "zpcii-disable", >>>> + machine_get_zpcii_disable, >> >> ^^ Here. >
diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c index 9134fe185f..5eb7fd12e2 100644 --- a/hw/s390x/s390-pci-kvm.c +++ b/hw/s390x/s390-pci-kvm.c @@ -22,7 +22,9 @@ bool s390_pci_kvm_interp_allowed(void) { - return kvm_s390_get_zpci_op() && !s390_is_pv(); + return (kvm_s390_get_zpci_op() && !s390_is_pv() && + !object_property_get_bool(OBJECT(qdev_get_machine()), + "zpcii-disable", NULL)); } int s390_pci_kvm_aif_enable(S390PCIBusDevice *pbdev, ZpciFib *fib, bool assist) diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index cc3097bfee..70229b102b 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -645,6 +645,21 @@ static inline void machine_set_dea_key_wrap(Object *obj, bool value, ms->dea_key_wrap = value; } +static inline bool machine_get_zpcii_disable(Object *obj, Error **errp) +{ + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); + + return ms->zpcii_disable; +} + +static inline void machine_set_zpcii_disable(Object *obj, bool value, + Error **errp) +{ + S390CcwMachineState *ms = S390_CCW_MACHINE(obj); + + ms->zpcii_disable = value; +} + static S390CcwMachineClass *current_mc; /* @@ -740,6 +755,13 @@ static inline void s390_machine_initfn(Object *obj) "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted" " to upper case) to pass to machine loader, boot manager," " and guest kernel"); + + object_property_add_bool(obj, "zpcii-disable", + machine_get_zpcii_disable, + machine_set_zpcii_disable); + object_property_set_description(obj, "zpcii-disable", + "disable zPCI interpretation facilties"); + object_property_set_bool(obj, "zpcii-disable", false, NULL); } static const TypeInfo ccw_machine_info = { @@ -804,9 +826,11 @@ DEFINE_CCW_MACHINE(7_1, "7.1", true); static void ccw_machine_7_0_instance_options(MachineState *machine) { static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V7_0 }; + S390CcwMachineState *ms = S390_CCW_MACHINE(machine); ccw_machine_7_1_instance_options(machine); s390_set_qemu_cpu_model(0x8561, 15, 1, qemu_cpu_feat); + ms->zpcii_disable = true; } static void ccw_machine_7_0_class_options(MachineClass *mc) diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h index 3331990e02..8a0090a071 100644 --- a/include/hw/s390x/s390-virtio-ccw.h +++ b/include/hw/s390x/s390-virtio-ccw.h @@ -27,6 +27,7 @@ struct S390CcwMachineState { bool aes_key_wrap; bool dea_key_wrap; bool pv; + bool zpcii_disable; uint8_t loadparm[8]; }; diff --git a/qemu-options.hx b/qemu-options.hx index 60cf188da4..fafe335b4a 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -36,7 +36,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ " nvdimm=on|off controls NVDIMM support (default=off)\n" " memory-encryption=@var{} memory encryption object to use (default=none)\n" " hmat=on|off controls ACPI HMAT support (default=off)\n" - " memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n", + " memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n" + " zpcii-disable=on|off disables zPCI interpretation facilities (default=off)\n", QEMU_ARCH_ALL) SRST ``-machine [type=]name[,prop=value[,...]]`` @@ -124,6 +125,11 @@ SRST -object memory-backend-ram,id=pc.ram,size=512M,x-use-canonical-path-for-ramblock-id=off -machine memory-backend=pc.ram -m 512M + + ``zpcii-disable=on|off`` + Disables zPCI interpretation facilties on s390-ccw hosts. + This feature can be used to disable hardware virtual assists + related to zPCI devices. The default is off. ERST DEF("M", HAS_ARG, QEMU_OPTION_M, diff --git a/util/qemu-config.c b/util/qemu-config.c index 433488aa56..5325f6bf80 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -236,6 +236,10 @@ static QemuOptsList machine_opts = { .help = "Up to 8 chars in set of [A-Za-z0-9. ](lower case chars" " converted to upper case) to pass to machine" " loader, boot manager, and guest kernel", + },{ + .name = "zpcii-disable", + .type = QEMU_OPT_BOOL, + .help = "disable zPCI interpretation facilities", }, { /* End of list */ } }
The zpcii-disable machine property can be used to force-disable the use of zPCI interpretation facilities for a VM. By default, this setting will be off for machine 7.1 and newer. Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> --- hw/s390x/s390-pci-kvm.c | 4 +++- hw/s390x/s390-virtio-ccw.c | 24 ++++++++++++++++++++++++ include/hw/s390x/s390-virtio-ccw.h | 1 + qemu-options.hx | 8 +++++++- util/qemu-config.c | 4 ++++ 5 files changed, 39 insertions(+), 2 deletions(-)