* [PATCH 0/1] vfio-ccw: Enable transparent CCW IPL from DASD
@ 2020-04-17 18:38 Jared Rossi
2020-04-17 18:38 ` [PATCH 1/1] " Jared Rossi
0 siblings, 1 reply; 6+ messages in thread
From: Jared Rossi @ 2020-04-17 18:38 UTC (permalink / raw)
To: Eric Farman, Cornelia Huck; +Cc: qemu-s390x, qemu-devel
Remove the explicit prefetch check when using vfio-ccw devices.
This check is not needed as all Linux channel programs are intended
to use prefetch and will be executed in the same way regardless.
Jared Rossi (1):
vfio-ccw: Enable transparent CCW IPL from DASD
hw/vfio/ccw.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
--
2.21.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
2020-04-17 18:38 [PATCH 0/1] vfio-ccw: Enable transparent CCW IPL from DASD Jared Rossi
@ 2020-04-17 18:38 ` Jared Rossi
2020-04-20 12:26 ` Cornelia Huck
0 siblings, 1 reply; 6+ messages in thread
From: Jared Rossi @ 2020-04-17 18:38 UTC (permalink / raw)
To: Eric Farman, Cornelia Huck; +Cc: qemu-s390x, qemu-devel
Remove the explicit prefetch check when using vfio-ccw devices.
This check is not needed as all Linux channel programs are intended
to use prefetch and will be executed in the same way regardless.
Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
hw/vfio/ccw.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 50cc2ec75c..e649377b68 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -74,16 +74,9 @@ static IOInstEnding vfio_ccw_handle_request(SubchDev *sch)
struct ccw_io_region *region = vcdev->io_region;
int ret;
- if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH)) {
- if (!(vcdev->force_orb_pfch)) {
- warn_once_pfch(vcdev, sch, "requires PFCH flag set");
- sch_gen_unit_exception(sch);
- css_inject_io_interrupt(sch);
- return IOINST_CC_EXPECTED;
- } else {
- sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
- warn_once_pfch(vcdev, sch, "PFCH flag forced");
- }
+ if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH) && vcdev->force_orb_pfch) {
+ sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
+ warn_once_pfch(vcdev, sch, "PFCH flag forced");
}
QEMU_BUILD_BUG_ON(sizeof(region->orb_area) != sizeof(ORB));
--
2.21.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
2020-04-17 18:38 ` [PATCH 1/1] " Jared Rossi
@ 2020-04-20 12:26 ` Cornelia Huck
2020-04-20 12:29 ` Cornelia Huck
0 siblings, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2020-04-20 12:26 UTC (permalink / raw)
To: Jared Rossi; +Cc: Eric Farman, qemu-s390x, qemu-devel
On Fri, 17 Apr 2020 14:38:38 -0400
Jared Rossi <jrossi@linux.ibm.com> wrote:
> Remove the explicit prefetch check when using vfio-ccw devices.
> This check is not needed as all Linux channel programs are intended
> to use prefetch and will be executed in the same way regardless.
As already commented on the Linux patch: Can we log something, so this
is debuggable if this statement does not hold true in the future?
>
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
> hw/vfio/ccw.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 50cc2ec75c..e649377b68 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -74,16 +74,9 @@ static IOInstEnding vfio_ccw_handle_request(SubchDev *sch)
> struct ccw_io_region *region = vcdev->io_region;
> int ret;
>
> - if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH)) {
> - if (!(vcdev->force_orb_pfch)) {
> - warn_once_pfch(vcdev, sch, "requires PFCH flag set");
> - sch_gen_unit_exception(sch);
> - css_inject_io_interrupt(sch);
> - return IOINST_CC_EXPECTED;
> - } else {
> - sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
> - warn_once_pfch(vcdev, sch, "PFCH flag forced");
> - }
> + if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH) && vcdev->force_orb_pfch) {
> + sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
> + warn_once_pfch(vcdev, sch, "PFCH flag forced");
> }
What happens when you run it with an old kernel? I guess the I/O is
only rejected later (after a trip into the kernel), but has that path
ever been tested?
>
> QEMU_BUILD_BUG_ON(sizeof(region->orb_area) != sizeof(ORB));
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
2020-04-20 12:26 ` Cornelia Huck
@ 2020-04-20 12:29 ` Cornelia Huck
2020-04-20 22:35 ` Jared Rossi
0 siblings, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2020-04-20 12:29 UTC (permalink / raw)
To: Jared Rossi; +Cc: Eric Farman, qemu-s390x, qemu-devel
On Mon, 20 Apr 2020 14:26:17 +0200
Cornelia Huck <cohuck@redhat.com> wrote:
> On Fri, 17 Apr 2020 14:38:38 -0400
> Jared Rossi <jrossi@linux.ibm.com> wrote:
>
> > Remove the explicit prefetch check when using vfio-ccw devices.
> > This check is not needed as all Linux channel programs are intended
> > to use prefetch and will be executed in the same way regardless.
>
> As already commented on the Linux patch: Can we log something, so this
> is debuggable if this statement does not hold true in the future?
>
> >
> > Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> > ---
> > hw/vfio/ccw.c | 13 +++----------
> > 1 file changed, 3 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> > index 50cc2ec75c..e649377b68 100644
> > --- a/hw/vfio/ccw.c
> > +++ b/hw/vfio/ccw.c
> > @@ -74,16 +74,9 @@ static IOInstEnding vfio_ccw_handle_request(SubchDev *sch)
> > struct ccw_io_region *region = vcdev->io_region;
> > int ret;
> >
> > - if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH)) {
> > - if (!(vcdev->force_orb_pfch)) {
> > - warn_once_pfch(vcdev, sch, "requires PFCH flag set");
> > - sch_gen_unit_exception(sch);
> > - css_inject_io_interrupt(sch);
> > - return IOINST_CC_EXPECTED;
> > - } else {
> > - sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
> > - warn_once_pfch(vcdev, sch, "PFCH flag forced");
> > - }
> > + if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH) && vcdev->force_orb_pfch) {
> > + sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
> > + warn_once_pfch(vcdev, sch, "PFCH flag forced");
> > }
>
> What happens when you run it with an old kernel? I guess the I/O is
> only rejected later (after a trip into the kernel), but has that path
> ever been tested?
>
> >
> > QEMU_BUILD_BUG_ON(sizeof(region->orb_area) != sizeof(ORB));
>
Oh, and do we want to deprecate the force prefetch interface in the
future? We probably need to wait a bit, until the kernel changes have
become widely available.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
2020-04-20 12:29 ` Cornelia Huck
@ 2020-04-20 22:35 ` Jared Rossi
2020-04-21 8:29 ` Cornelia Huck
0 siblings, 1 reply; 6+ messages in thread
From: Jared Rossi @ 2020-04-20 22:35 UTC (permalink / raw)
To: Cornelia Huck; +Cc: Eric Farman, qemu-s390x, qemu-devel
On 2020-04-20 08:29, Cornelia Huck wrote:
> On Mon, 20 Apr 2020 14:26:17 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
>> On Fri, 17 Apr 2020 14:38:38 -0400
>> Jared Rossi <jrossi@linux.ibm.com> wrote:
>>
>> > Remove the explicit prefetch check when using vfio-ccw devices.
>> > This check is not needed as all Linux channel programs are intended
>> > to use prefetch and will be executed in the same way regardless.
>>
>> As already commented on the Linux patch: Can we log something, so this
>> is debuggable if this statement does not hold true in the future?
>>
Agreed. I will work on debugging improvements so that any future issues
related to unintended prefetching are more clearly logged.
>> >
>> > Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
>> > ---
>> > hw/vfio/ccw.c | 13 +++----------
>> > 1 file changed, 3 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
>> > index 50cc2ec75c..e649377b68 100644
>> > --- a/hw/vfio/ccw.c
>> > +++ b/hw/vfio/ccw.c
>> > @@ -74,16 +74,9 @@ static IOInstEnding vfio_ccw_handle_request(SubchDev *sch)
>> > struct ccw_io_region *region = vcdev->io_region;
>> > int ret;
>> >
>> > - if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH)) {
>> > - if (!(vcdev->force_orb_pfch)) {
>> > - warn_once_pfch(vcdev, sch, "requires PFCH flag set");
>> > - sch_gen_unit_exception(sch);
>> > - css_inject_io_interrupt(sch);
>> > - return IOINST_CC_EXPECTED;
>> > - } else {
>> > - sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
>> > - warn_once_pfch(vcdev, sch, "PFCH flag forced");
>> > - }
>> > + if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH) && vcdev->force_orb_pfch) {
>> > + sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
>> > + warn_once_pfch(vcdev, sch, "PFCH flag forced");
>> > }
>>
>> What happens when you run it with an old kernel? I guess the I/O is
>> only rejected later (after a trip into the kernel), but has that path
>> ever been tested?
>>
Yes, this was tested and you are correct that the kernel will reject the
I/O unless
the corresponding patch is also applied there. I will revisit this path
while I'm
updating the logging to ensure that any potential interactions are
appropriately
considered.
>> >
>> > QEMU_BUILD_BUG_ON(sizeof(region->orb_area) != sizeof(ORB));
>>
>
> Oh, and do we want to deprecate the force prefetch interface in the
> future? We probably need to wait a bit, until the kernel changes have
> become widely available.
Yes, I think we will want to deprecate it at an appropriate time in the
future.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] vfio-ccw: Enable transparent CCW IPL from DASD
2020-04-20 22:35 ` Jared Rossi
@ 2020-04-21 8:29 ` Cornelia Huck
0 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2020-04-21 8:29 UTC (permalink / raw)
To: Jared Rossi; +Cc: Eric Farman, qemu-s390x, qemu-devel
On Mon, 20 Apr 2020 18:35:58 -0400
Jared Rossi <jrossi@linux.ibm.com> wrote:
> On 2020-04-20 08:29, Cornelia Huck wrote:
> > On Mon, 20 Apr 2020 14:26:17 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:
> >
> >> On Fri, 17 Apr 2020 14:38:38 -0400
> >> Jared Rossi <jrossi@linux.ibm.com> wrote:
> >>
> >> > Remove the explicit prefetch check when using vfio-ccw devices.
> >> > This check is not needed as all Linux channel programs are intended
> >> > to use prefetch and will be executed in the same way regardless.
> >>
> >> As already commented on the Linux patch: Can we log something, so this
> >> is debuggable if this statement does not hold true in the future?
> >>
>
> Agreed. I will work on debugging improvements so that any future issues
> related to unintended prefetching are more clearly logged.
Great.
>
> >> >
> >> > Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> >> > ---
> >> > hw/vfio/ccw.c | 13 +++----------
> >> > 1 file changed, 3 insertions(+), 10 deletions(-)
> >> >
> >> > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> >> > index 50cc2ec75c..e649377b68 100644
> >> > --- a/hw/vfio/ccw.c
> >> > +++ b/hw/vfio/ccw.c
> >> > @@ -74,16 +74,9 @@ static IOInstEnding vfio_ccw_handle_request(SubchDev *sch)
> >> > struct ccw_io_region *region = vcdev->io_region;
> >> > int ret;
> >> >
> >> > - if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH)) {
> >> > - if (!(vcdev->force_orb_pfch)) {
> >> > - warn_once_pfch(vcdev, sch, "requires PFCH flag set");
> >> > - sch_gen_unit_exception(sch);
> >> > - css_inject_io_interrupt(sch);
> >> > - return IOINST_CC_EXPECTED;
> >> > - } else {
> >> > - sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
> >> > - warn_once_pfch(vcdev, sch, "PFCH flag forced");
> >> > - }
> >> > + if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH) && vcdev->force_orb_pfch) {
> >> > + sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
> >> > + warn_once_pfch(vcdev, sch, "PFCH flag forced");
> >> > }
> >>
> >> What happens when you run it with an old kernel? I guess the I/O is
> >> only rejected later (after a trip into the kernel), but has that path
> >> ever been tested?
> >>
>
> Yes, this was tested and you are correct that the kernel will reject the
> I/O unless
> the corresponding patch is also applied there. I will revisit this path
> while I'm
> updating the logging to ensure that any potential interactions are
> appropriately
> considered.
I've looked at the code again and it seems that the kernel will end up
signaling -EOPNOTSUPP to us on that case, which causes the same unit
exception as without this patch, so we should be all good.
>
> >> >
> >> > QEMU_BUILD_BUG_ON(sizeof(region->orb_area) != sizeof(ORB));
> >>
> >
> > Oh, and do we want to deprecate the force prefetch interface in the
> > future? We probably need to wait a bit, until the kernel changes have
> > become widely available.
>
> Yes, I think we will want to deprecate it at an appropriate time in the
> future.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-21 8:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-17 18:38 [PATCH 0/1] vfio-ccw: Enable transparent CCW IPL from DASD Jared Rossi
2020-04-17 18:38 ` [PATCH 1/1] " Jared Rossi
2020-04-20 12:26 ` Cornelia Huck
2020-04-20 12:29 ` Cornelia Huck
2020-04-20 22:35 ` Jared Rossi
2020-04-21 8:29 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).