Re: [PATCH v2 9/9] xue: allow driving the rest of XHCI by a domain while Xen uses DbC

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.07.2022 17:32, Marek Marczykowski-Górecki wrote:
> That's possible, because the capability was designed specifically to
> allow separate driver handle it, in parallel to unmodified xhci driver
> (separate set of registers, pretending the port is "disconnected" for
> the main xhci driver etc). It works with Linux dom0, although requires
> an awful hack - re-enabling bus mastering behind dom0's backs.
> Linux driver does similar thing - see
> drivers/usb/early/xhci-dbc.c:xdbc_handle_events().

Isn't there a risk that intermediately data was lost?

> To avoid Linux messing with the DbC, mark this MMIO area as read-only.

In principle this would want to happen quite a bit earlier in the
series. I'm okay with it being kept here as long as it is made
very obvious to and easily noticeable by committers that this series
should only be committed all in one go.

Also along with this is where I'd see the pci_hide_device() go.

> @@ -817,6 +819,16 @@ static void xue_flush(struct xue *xue, struct 
> xue_trb_ring *trb,
>          xue_enable_dbc(xue);
>      }
>  
> +    /* Re-enable bus mastering, if dom0 (or other) disabled it in the 
> meantime. */
> +    cmd = pci_conf_read16(xue->sbdf, PCI_COMMAND);
> +#define XUE_XHCI_CMD_REQUIRED (PCI_COMMAND_MEMORY|PCI_COMMAND_MASTER)
> +    if ( (cmd & XUE_XHCI_CMD_REQUIRED) != XUE_XHCI_CMD_REQUIRED )
> +    {
> +        cmd |= XUE_XHCI_CMD_REQUIRED;
> +        pci_conf_write16(xue->sbdf, PCI_COMMAND, cmd);
> +    }
> +#undef XUE_XHCI_CMD_REQUIRED
> +
>      xue_pop_events(xue);
>  
>      if ( !(reg->ctrl & (1UL << XUE_CTRL_DCR)) )
> @@ -916,6 +928,13 @@ static void __init cf_check xue_uart_init_postirq(struct 
> serial_port *port)
>      serial_async_transmit(port);
>      init_timer(&uart->timer, xue_uart_poll, port, 0);
>      set_timer(&uart->timer, NOW() + MILLISECS(1));
> +
> +#ifdef CONFIG_X86
> +    if ( rangeset_add_range(mmio_ro_ranges,
> +                PFN_DOWN(uart->xue.xhc_mmio_phys + uart->xue.xhc_dbc_offset),
> +                PFN_UP(uart->xue.xhc_mmio_phys + uart->xue.xhc_dbc_offset + 
> sizeof(*uart->xue.dbc_reg)) - 1) )
> +        printk(XENLOG_INFO "Error while adding MMIO range of device to 
> mmio_ro_ranges\n");
> +#endif
>  }

According to my counting there are three overly long lines in these
two hunks.

Jan