Re: [PATCH v2 2/2] drivers/char: Use sub-page ro API to make just xhci dbc cap RO

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.05.2023 23:25, Marek Marczykowski-Górecki wrote:
> Not the whole page, which may contain other registers too. In fact
> on Tiger Lake and newer (at least), this page do contain other registers
> that Linux tries to use.

Please can you clarify whether this is with spec or an erratum? I ask
not the least because I continue to wonder whether we really want/need
the non-negligible amount of new code added by path 1.

> And with share=yes, a domU would use them too.

And gain yet more access to the emulator, as mentioned in patch 1. The
security implications may (will?) want mentioning.

> --- a/xen/drivers/char/xhci-dbc.c
> +++ b/xen/drivers/char/xhci-dbc.c
> @@ -1221,14 +1221,12 @@ static void __init cf_check 
> dbc_uart_init_postirq(struct serial_port *port)
>       * Linux's XHCI driver (as of 5.18) works without writting to the whole
>       * page, so keep it simple.
>       */
> -    if ( rangeset_add_range(mmio_ro_ranges,
> -                PFN_DOWN((uart->dbc.bar_val & PCI_BASE_ADDRESS_MEM_MASK) +
> -                         uart->dbc.xhc_dbc_offset),
> -                PFN_UP((uart->dbc.bar_val & PCI_BASE_ADDRESS_MEM_MASK) +
> -                       uart->dbc.xhc_dbc_offset +
> -                sizeof(*uart->dbc.dbc_reg)) - 1) )
> -        printk(XENLOG_INFO
> -               "Error while adding MMIO range of device to 
> mmio_ro_ranges\n");
> +    if ( subpage_mmio_ro_add(
> +            (uart->dbc.bar_val & PCI_BASE_ADDRESS_MEM_MASK) +
> +             uart->dbc.xhc_dbc_offset,
> +            sizeof(*uart->dbc.dbc_reg)) )
> +        printk(XENLOG_WARNING
> +               "Error while marking MMIO range of XHCI console as R/O\n");

So how about falling back to just rangeset_add_range(mmio_ro_ranges, ...)
in this failure case? (I did mention an alternative to doing it here in
the comments on patch 1.)

Also, doesn't the comment ahead of the construct become stale?

Finally I think indentation of the function call arguments is off by one.

Jan