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
