mbox series

[v2,0/2] Add API for making parts of a MMIO page R/O and use it in XHCI console

Message ID cover.dd82aca339854e90ffe12e7bc4298254a6caaf0d.1683321183.git-series.marmarek@invisiblethingslab.com (mailing list archive)
Headers show
Series Add API for making parts of a MMIO page R/O and use it in XHCI console | expand

Message

Marek Marczykowski-Górecki May 5, 2023, 9:25 p.m. UTC
On older systems, XHCI xcap had a layout that no other (interesting) registers
were placed on the same page as the debug capability, so Linux was fine with
making the whole page R/O. But at least on Tiger Lake and Alder Lake, Linux
needs to write to some other registers on the same page too.

Add a generic API for making just parts of an MMIO page R/O and use it to fix
USB3 console with share=yes or share=hwdom options. More details in commit
messages.

Marek Marczykowski-Górecki (2):
  x86/mm: add API for marking only part of a MMIO page read only
  drivers/char: Use sub-page ro API to make just xhci dbc cap RO

 xen/arch/x86/hvm/emulate.c      |   2 +-
 xen/arch/x86/hvm/hvm.c          |   8 +-
 xen/arch/x86/include/asm/mm.h   |  15 ++-
 xen/arch/x86/mm.c               | 253 +++++++++++++++++++++++++++++++++-
 xen/arch/x86/pv/ro-page-fault.c |   1 +-
 xen/drivers/char/xhci-dbc.c     |  14 +--
 6 files changed, 284 insertions(+), 9 deletions(-)

base-commit: 99a9c3d7141063ae3f357892c6181cfa3be8a280

Comments

Jason Andryuk May 11, 2023, 2:58 p.m. UTC | #1
On Fri, May 5, 2023 at 5:26 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On older systems, XHCI xcap had a layout that no other (interesting) registers
> were placed on the same page as the debug capability, so Linux was fine with
> making the whole page R/O. But at least on Tiger Lake and Alder Lake, Linux
> needs to write to some other registers on the same page too.
>
> Add a generic API for making just parts of an MMIO page R/O and use it to fix
> USB3 console with share=yes or share=hwdom options. More details in commit
> messages.
>
> Marek Marczykowski-Górecki (2):
>   x86/mm: add API for marking only part of a MMIO page read only
>   drivers/char: Use sub-page ro API to make just xhci dbc cap RO

Series:
Tested-by: Jason Andryuk <jandryuk@gmail.com>

I had the issue with a 10th Gen, Comet Lake, laptop.  With an HVM
usbvm with dbgp=xhci,share=1, Xen crashed the domain because of:
(XEN) d1v0 EPT violation 0xdaa (-w-/r-x) gpa 0x000000f1008470 mfn 0xcc328 type 5

The BAR is mfn 0xcc320-0xcc32f

Booting PV, it faulted at drivers/usb/host/pci-quirks.c:1170 which looks to be:
```
/* Disable any BIOS SMIs and clear all SMI events*/
writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
```

Thanks for integrating XUE, Marek!

Regards,
Jason
Marek Marczykowski-Górecki May 11, 2023, 3:22 p.m. UTC | #2
On Thu, May 11, 2023 at 10:58:48AM -0400, Jason Andryuk wrote:
> On Fri, May 5, 2023 at 5:26 PM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> >
> > On older systems, XHCI xcap had a layout that no other (interesting) registers
> > were placed on the same page as the debug capability, so Linux was fine with
> > making the whole page R/O. But at least on Tiger Lake and Alder Lake, Linux
> > needs to write to some other registers on the same page too.
> >
> > Add a generic API for making just parts of an MMIO page R/O and use it to fix
> > USB3 console with share=yes or share=hwdom options. More details in commit
> > messages.
> >
> > Marek Marczykowski-Górecki (2):
> >   x86/mm: add API for marking only part of a MMIO page read only
> >   drivers/char: Use sub-page ro API to make just xhci dbc cap RO
> 
> Series:
> Tested-by: Jason Andryuk <jandryuk@gmail.com>
> 
> I had the issue with a 10th Gen, Comet Lake, laptop.  With an HVM
> usbvm with dbgp=xhci,share=1, Xen crashed the domain because of:
> (XEN) d1v0 EPT violation 0xdaa (-w-/r-x) gpa 0x000000f1008470 mfn 0xcc328 type 5

Hmm, this series is supposed to avoid exactly this issue. I tested it on
12th Gen, so maybe 10th Gen has a bit different layout.
Can you add a debug print before subpage_mmio_ro_add() call in
xhci-dbc.c and see what area is getting protected?

> The BAR is mfn 0xcc320-0xcc32f
> 
> Booting PV, it faulted at drivers/usb/host/pci-quirks.c:1170 which looks to be:
> ```
> /* Disable any BIOS SMIs and clear all SMI events*/
> writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
> ```
> 
> Thanks for integrating XUE, Marek!
Jason Andryuk May 11, 2023, 3:27 p.m. UTC | #3
On Thu, May 11, 2023 at 11:22 AM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
>
> On Thu, May 11, 2023 at 10:58:48AM -0400, Jason Andryuk wrote:
> > On Fri, May 5, 2023 at 5:26 PM Marek Marczykowski-Górecki
> > <marmarek@invisiblethingslab.com> wrote:
> > >
> > > On older systems, XHCI xcap had a layout that no other (interesting) registers
> > > were placed on the same page as the debug capability, so Linux was fine with
> > > making the whole page R/O. But at least on Tiger Lake and Alder Lake, Linux
> > > needs to write to some other registers on the same page too.
> > >
> > > Add a generic API for making just parts of an MMIO page R/O and use it to fix
> > > USB3 console with share=yes or share=hwdom options. More details in commit
> > > messages.
> > >
> > > Marek Marczykowski-Górecki (2):
> > >   x86/mm: add API for marking only part of a MMIO page read only
> > >   drivers/char: Use sub-page ro API to make just xhci dbc cap RO
> >
> > Series:
> > Tested-by: Jason Andryuk <jandryuk@gmail.com>
> >
> > I had the issue with a 10th Gen, Comet Lake, laptop.  With an HVM
> > usbvm with dbgp=xhci,share=1, Xen crashed the domain because of:
> > (XEN) d1v0 EPT violation 0xdaa (-w-/r-x) gpa 0x000000f1008470 mfn 0xcc328 type 5
>
> Hmm, this series is supposed to avoid exactly this issue. I tested it on
> 12th Gen, so maybe 10th Gen has a bit different layout.
> Can you add a debug print before subpage_mmio_ro_add() call in
> xhci-dbc.c and see what area is getting protected?

Your series fixes the problem!

I just had the details from the original issue, so I included them.  I
was trying to highlight what this series fixed for me.  Sorry for the
confusion.

Regards,
Jason
Marek Marczykowski-Górecki May 11, 2023, 3:28 p.m. UTC | #4
On Thu, May 11, 2023 at 11:27:02AM -0400, Jason Andryuk wrote:
> On Thu, May 11, 2023 at 11:22 AM Marek Marczykowski-Górecki
> <marmarek@invisiblethingslab.com> wrote:
> >
> > On Thu, May 11, 2023 at 10:58:48AM -0400, Jason Andryuk wrote:
> > > On Fri, May 5, 2023 at 5:26 PM Marek Marczykowski-Górecki
> > > <marmarek@invisiblethingslab.com> wrote:
> > > >
> > > > On older systems, XHCI xcap had a layout that no other (interesting) registers
> > > > were placed on the same page as the debug capability, so Linux was fine with
> > > > making the whole page R/O. But at least on Tiger Lake and Alder Lake, Linux
> > > > needs to write to some other registers on the same page too.
> > > >
> > > > Add a generic API for making just parts of an MMIO page R/O and use it to fix
> > > > USB3 console with share=yes or share=hwdom options. More details in commit
> > > > messages.
> > > >
> > > > Marek Marczykowski-Górecki (2):
> > > >   x86/mm: add API for marking only part of a MMIO page read only
> > > >   drivers/char: Use sub-page ro API to make just xhci dbc cap RO
> > >
> > > Series:
> > > Tested-by: Jason Andryuk <jandryuk@gmail.com>
> > >
> > > I had the issue with a 10th Gen, Comet Lake, laptop.  With an HVM
> > > usbvm with dbgp=xhci,share=1, Xen crashed the domain because of:
> > > (XEN) d1v0 EPT violation 0xdaa (-w-/r-x) gpa 0x000000f1008470 mfn 0xcc328 type 5
> >
> > Hmm, this series is supposed to avoid exactly this issue. I tested it on
> > 12th Gen, so maybe 10th Gen has a bit different layout.
> > Can you add a debug print before subpage_mmio_ro_add() call in
> > xhci-dbc.c and see what area is getting protected?
> 
> Your series fixes the problem!
> 
> I just had the details from the original issue, so I included them.  I
> was trying to highlight what this series fixed for me.  Sorry for the
> confusion.

Ah, then all is good, thanks for testing! :)