Re: [PATCH v2 1/1] vfio-ccw: allow non-prefetch ORBs

[Cornelia Huck]

On Tue, 12 May 2020 14:15:35 -0400
Jared Rossi <address@hidden> wrote:

> Remove the explicit prefetch check when using vfio-ccw devices.
> This check does not trigger in practice as all Linux channel programs
> are intended to use prefetch.
> 
> It is no longer required to force the PFCH flag when using vfio-ccw
> devices.

That's not quite true: Only kernels that include the currently-queued
patch do not require it. Maybe

"Newer Linux kernel versions do not require to force the PFCH flag with
vfio-ccw devices anymore."

?

> 
> Signed-off-by: Jared Rossi <address@hidden>
> ---
>  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));

Let me spell out what happens:
- PFCH bit set -> no change
- PFCH bit not set, but force_orb_pfch set -> no change
- neither PFCH bit nor force_orb_pfch set:
  - older kernels: QEMU makes the request, the kernel rejects it, guest
    gets a unit exception (same result for the guest as before, only a
    different code flow)
  - newer kernels: QEMU makes the request, the kernel forwards the
    request (logging a rate-limited warning); the result depends on
    whether the guest actually tries to rewrite the channel program or
    not

I think that is what we want, and I think I'll queue this patch with
the tweaked commit message, but I'd like a second opinion.

(We should also deprecate force_orb_pfch in the future.)