[PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers

Jan Beulich posted 8 patches 7 months ago
There is a newer version of this series
[PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Jan Beulich 7 months ago
In preparation of dropping the register parameters from
serial_[rt]x_interrupt() and in turn from IRQ handler functions,
register state needs making available another way for the few key
handlers which need it. Fake IRQ-like state.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
other console poll functions we have, and it's unclear whether that's
actually generally correct.

Andrew suggested to move set_irq_regs() to BUGFRAME_run_fn handling;
it's not clear to me whether that would be (a) correct from an abstract
pov (that's exception, not interrupt context after all) and (b) really
beneficial.
---
v2: New.

--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1253,6 +1253,7 @@ static void cf_check _ehci_dbgp_poll(str
     unsigned long flags;
     unsigned int timeout = MICROSECS(DBGP_CHECK_INTERVAL);
     bool empty = false;
+    struct cpu_user_regs *old_regs;
 
     if ( !dbgp->ehci_debug )
         return;
@@ -1268,12 +1269,17 @@ static void cf_check _ehci_dbgp_poll(str
         spin_unlock_irqrestore(&port->tx_lock, flags);
     }
 
+    /* Mimic interrupt context. */
+    old_regs = set_irq_regs(regs);
+
     if ( dbgp->in.chunk )
         serial_rx_interrupt(port, regs);
 
     if ( empty )
         serial_tx_interrupt(port, regs);
 
+    set_irq_regs(old_regs);
+
     if ( spin_trylock_irqsave(&port->tx_lock, flags) )
     {
         if ( dbgp->state == dbgp_idle && !dbgp->in.chunk &&
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -211,10 +211,14 @@ static void cf_check __ns16550_poll(stru
 {
     struct serial_port *port = this_cpu(poll_port);
     struct ns16550 *uart = port->uart;
+    struct cpu_user_regs *old_regs;
 
     if ( uart->intr_works )
         return; /* Interrupts work - no more polling */
 
+    /* Mimic interrupt context. */
+    old_regs = set_irq_regs(regs);
+
     while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
     {
         if ( ns16550_ioport_invalid(uart) )
@@ -227,6 +231,7 @@ static void cf_check __ns16550_poll(stru
         serial_tx_interrupt(port, regs);
 
 out:
+    set_irq_regs(old_regs);
     set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
 }
 
--- a/xen/drivers/char/xhci-dbc.c
+++ b/xen/drivers/char/xhci-dbc.c
@@ -1164,6 +1164,7 @@ static void cf_check dbc_uart_poll(void
     struct dbc_uart *uart = port->uart;
     struct dbc *dbc = &uart->dbc;
     unsigned long flags = 0;
+    struct cpu_user_regs *old_regs;
 
     if ( spin_trylock_irqsave(&port->tx_lock, flags) )
     {
@@ -1175,10 +1176,15 @@ static void cf_check dbc_uart_poll(void
         spin_unlock_irqrestore(&port->tx_lock, flags);
     }
 
+    /* Mimic interrupt context. */
+    old_regs = set_irq_regs(guest_cpu_user_regs());
+
     while ( dbc_work_ring_size(&dbc->dbc_iwork) )
         serial_rx_interrupt(port, guest_cpu_user_regs());
 
     serial_tx_interrupt(port, guest_cpu_user_regs());
+
+    set_irq_regs(old_regs);
     set_timer(&uart->timer, NOW() + MICROSECS(DBC_POLL_INTERVAL));
 }
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Jan Beulich 6 months, 3 weeks ago
On 05.02.2024 14:27, Jan Beulich wrote:
> In preparation of dropping the register parameters from
> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
> register state needs making available another way for the few key
> handlers which need it. Fake IRQ-like state.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
> other console poll functions we have, and it's unclear whether that's
> actually generally correct.

It occurs to me that due to this behavior, ...

> --- a/xen/drivers/char/xhci-dbc.c
> +++ b/xen/drivers/char/xhci-dbc.c
> @@ -1164,6 +1164,7 @@ static void cf_check dbc_uart_poll(void
>      struct dbc_uart *uart = port->uart;
>      struct dbc *dbc = &uart->dbc;
>      unsigned long flags = 0;
> +    struct cpu_user_regs *old_regs;
>  
>      if ( spin_trylock_irqsave(&port->tx_lock, flags) )
>      {
> @@ -1175,10 +1176,15 @@ static void cf_check dbc_uart_poll(void
>          spin_unlock_irqrestore(&port->tx_lock, flags);
>      }
>  
> +    /* Mimic interrupt context. */
> +    old_regs = set_irq_regs(guest_cpu_user_regs());
> +
>      while ( dbc_work_ring_size(&dbc->dbc_iwork) )
>          serial_rx_interrupt(port, guest_cpu_user_regs());
>  
>      serial_tx_interrupt(port, guest_cpu_user_regs());
> +
> +    set_irq_regs(old_regs);
>      set_timer(&uart->timer, NOW() + MICROSECS(DBC_POLL_INTERVAL));
>  }
>  
> 

... this ought to be unnecessary, considering what dump_registers()
is changed to in patch 2.

Jan
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Julien Grall 7 months ago
Hi Jan,

On 05/02/2024 13:27, Jan Beulich wrote:
> In preparation of dropping the register parameters from
> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
> register state needs making available another way for the few key
> handlers which need it. Fake IRQ-like state.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
> other console poll functions we have, and it's unclear whether that's
> actually generally correct.

Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
run_in_exception() doesn't exist. But looking at the caller, no-on seems 
to care about the 'regs'. So is this just a latent bug?

BTW, do you have an idea why the poll function is not run in an 
exception handler?

> 
> Andrew suggested to move set_irq_regs() to BUGFRAME_run_fn handling;
> it's not clear to me whether that would be (a) correct from an abstract
> pov (that's exception, not interrupt context after all) 

I agree with that.

> and (b) really beneficial.

I guess this could help to reduce the amount of churn. I can't really 
make my mind whether this is worth it or not. So I would keep it as you did.

Cheers,

-- 
Julien Grall
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Jan Beulich 6 months, 4 weeks ago
On 08.02.2024 23:00, Julien Grall wrote:
> On 05/02/2024 13:27, Jan Beulich wrote:
>> In preparation of dropping the register parameters from
>> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
>> register state needs making available another way for the few key
>> handlers which need it. Fake IRQ-like state.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
>> other console poll functions we have, and it's unclear whether that's
>> actually generally correct.
> 
> Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
> run_in_exception() doesn't exist. But looking at the caller, no-on seems 
> to care about the 'regs'. So is this just a latent bug?

What do you mean by "doesn't exist"? ns16550_poll() assumes it exists.
And I can spot any use of guest_user_regs() on the respective generic
or Arm-specific bug.c paths.

> BTW, do you have an idea why the poll function is not run in an 
> exception handler?

"The poll function" being which one? If you mean the one in xhci-dbc.c
then that's why I had Cc-ed Marek. Moving him to To: - maybe that
manages to finally catch his attention.

>> Andrew suggested to move set_irq_regs() to BUGFRAME_run_fn handling;
>> it's not clear to me whether that would be (a) correct from an abstract
>> pov (that's exception, not interrupt context after all) 
> 
> I agree with that.
> 
>> and (b) really beneficial.
> 
> I guess this could help to reduce the amount of churn. I can't really 
> make my mind whether this is worth it or not. So I would keep it as you did.

Good, thanks.

Jan
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Marek Marczykowski 6 months, 3 weeks ago
On Mon, Feb 12, 2024 at 10:04:38AM +0100, Jan Beulich wrote:
> On 08.02.2024 23:00, Julien Grall wrote:
> > On 05/02/2024 13:27, Jan Beulich wrote:
> >> In preparation of dropping the register parameters from
> >> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
> >> register state needs making available another way for the few key
> >> handlers which need it. Fake IRQ-like state.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> ---
> >> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
> >> other console poll functions we have, and it's unclear whether that's
> >> actually generally correct.
> > 
> > Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
> > run_in_exception() doesn't exist. But looking at the caller, no-on seems 
> > to care about the 'regs'. So is this just a latent bug?
> 
> What do you mean by "doesn't exist"? ns16550_poll() assumes it exists.
> And I can spot any use of guest_user_regs() on the respective generic
> or Arm-specific bug.c paths.
> 
> > BTW, do you have an idea why the poll function is not run in an 
> > exception handler?
> 
> "The poll function" being which one? If you mean the one in xhci-dbc.c
> then that's why I had Cc-ed Marek. Moving him to To: - maybe that
> manages to finally catch his attention.

TBH, I don't know. That's part of the original xue patch at
https://github.com/connojd/xue/blob/master/patches/xen-xue-dbgp.patch
and it works for me as it is.

> >> Andrew suggested to move set_irq_regs() to BUGFRAME_run_fn handling;
> >> it's not clear to me whether that would be (a) correct from an abstract
> >> pov (that's exception, not interrupt context after all) 
> > 
> > I agree with that.
> > 
> >> and (b) really beneficial.
> > 
> > I guess this could help to reduce the amount of churn. I can't really 
> > make my mind whether this is worth it or not. So I would keep it as you did.
> 
> Good, thanks.
> 
> Jan

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Jan Beulich 6 months, 3 weeks ago
On 13.02.2024 04:43, Marek Marczykowski wrote:
> On Mon, Feb 12, 2024 at 10:04:38AM +0100, Jan Beulich wrote:
>> On 08.02.2024 23:00, Julien Grall wrote:
>>> On 05/02/2024 13:27, Jan Beulich wrote:
>>>> In preparation of dropping the register parameters from
>>>> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
>>>> register state needs making available another way for the few key
>>>> handlers which need it. Fake IRQ-like state.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
>>>> other console poll functions we have, and it's unclear whether that's
>>>> actually generally correct.
>>>
>>> Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
>>> run_in_exception() doesn't exist. But looking at the caller, no-on seems 
>>> to care about the 'regs'. So is this just a latent bug?
>>
>> What do you mean by "doesn't exist"? ns16550_poll() assumes it exists.
>> And I can spot any use of guest_user_regs() on the respective generic
>> or Arm-specific bug.c paths.
>>
>>> BTW, do you have an idea why the poll function is not run in an 
>>> exception handler?
>>
>> "The poll function" being which one? If you mean the one in xhci-dbc.c
>> then that's why I had Cc-ed Marek. Moving him to To: - maybe that
>> manages to finally catch his attention.
> 
> TBH, I don't know. That's part of the original xue patch at
> https://github.com/connojd/xue/blob/master/patches/xen-xue-dbgp.patch
> and it works for me as it is.

"Works" meaning what? Doesn't crash on you? Or does also provide
sensible output in _all_ cases (i.e. including when e.g. the poll
happens to run on an idle vCPU)?

Jan
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Marek Marczykowski 6 months, 3 weeks ago
On Tue, Feb 13, 2024 at 08:45:54AM +0100, Jan Beulich wrote:
> On 13.02.2024 04:43, Marek Marczykowski wrote:
> > On Mon, Feb 12, 2024 at 10:04:38AM +0100, Jan Beulich wrote:
> >> On 08.02.2024 23:00, Julien Grall wrote:
> >>> On 05/02/2024 13:27, Jan Beulich wrote:
> >>>> In preparation of dropping the register parameters from
> >>>> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
> >>>> register state needs making available another way for the few key
> >>>> handlers which need it. Fake IRQ-like state.
> >>>>
> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>>> ---
> >>>> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
> >>>> other console poll functions we have, and it's unclear whether that's
> >>>> actually generally correct.
> >>>
> >>> Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
> >>> run_in_exception() doesn't exist. But looking at the caller, no-on seems 
> >>> to care about the 'regs'. So is this just a latent bug?
> >>
> >> What do you mean by "doesn't exist"? ns16550_poll() assumes it exists.
> >> And I can spot any use of guest_user_regs() on the respective generic
> >> or Arm-specific bug.c paths.
> >>
> >>> BTW, do you have an idea why the poll function is not run in an 
> >>> exception handler?
> >>
> >> "The poll function" being which one? If you mean the one in xhci-dbc.c
> >> then that's why I had Cc-ed Marek. Moving him to To: - maybe that
> >> manages to finally catch his attention.
> > 
> > TBH, I don't know. That's part of the original xue patch at
> > https://github.com/connojd/xue/blob/master/patches/xen-xue-dbgp.patch
> > and it works for me as it is.
> 
> "Works" meaning what? Doesn't crash on you? Or does also provide
> sensible output in _all_ cases (i.e. including when e.g. the poll
> happens to run on an idle vCPU)?

Generally provides sensible output, for example during boot (it is using
idle vCPU then, right?).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Jan Beulich 6 months, 3 weeks ago
On 13.02.2024 15:53, Marek Marczykowski wrote:
> On Tue, Feb 13, 2024 at 08:45:54AM +0100, Jan Beulich wrote:
>> On 13.02.2024 04:43, Marek Marczykowski wrote:
>>> On Mon, Feb 12, 2024 at 10:04:38AM +0100, Jan Beulich wrote:
>>>> On 08.02.2024 23:00, Julien Grall wrote:
>>>>> On 05/02/2024 13:27, Jan Beulich wrote:
>>>>>> In preparation of dropping the register parameters from
>>>>>> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
>>>>>> register state needs making available another way for the few key
>>>>>> handlers which need it. Fake IRQ-like state.
>>>>>>
>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>> ---
>>>>>> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
>>>>>> other console poll functions we have, and it's unclear whether that's
>>>>>> actually generally correct.
>>>>>
>>>>> Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
>>>>> run_in_exception() doesn't exist. But looking at the caller, no-on seems 
>>>>> to care about the 'regs'. So is this just a latent bug?
>>>>
>>>> What do you mean by "doesn't exist"? ns16550_poll() assumes it exists.
>>>> And I can spot any use of guest_user_regs() on the respective generic
>>>> or Arm-specific bug.c paths.
>>>>
>>>>> BTW, do you have an idea why the poll function is not run in an 
>>>>> exception handler?
>>>>
>>>> "The poll function" being which one? If you mean the one in xhci-dbc.c
>>>> then that's why I had Cc-ed Marek. Moving him to To: - maybe that
>>>> manages to finally catch his attention.
>>>
>>> TBH, I don't know. That's part of the original xue patch at
>>> https://github.com/connojd/xue/blob/master/patches/xen-xue-dbgp.patch
>>> and it works for me as it is.
>>
>> "Works" meaning what? Doesn't crash on you? Or does also provide
>> sensible output in _all_ cases (i.e. including when e.g. the poll
>> happens to run on an idle vCPU)?
> 
> Generally provides sensible output, for example during boot (it is using
> idle vCPU then, right?).

Before Dom0 is started: Yes. With the exception of the phase where PV
Dom0's page tables are constructed, albeit in that time window
guest_cpu_user_regs() shouldn't yield sensible data either. I can only
say I'm surprised; since I have no way to properly test with an XHCI
debug port, I'd have to see about faking something to convince myself
(unless you were to supply example output).

Jan
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Marek Marczykowski 6 months, 3 weeks ago
On Tue, Feb 13, 2024 at 04:00:32PM +0100, Jan Beulich wrote:
> On 13.02.2024 15:53, Marek Marczykowski wrote:
> > On Tue, Feb 13, 2024 at 08:45:54AM +0100, Jan Beulich wrote:
> >> On 13.02.2024 04:43, Marek Marczykowski wrote:
> >>> On Mon, Feb 12, 2024 at 10:04:38AM +0100, Jan Beulich wrote:
> >>>> On 08.02.2024 23:00, Julien Grall wrote:
> >>>>> On 05/02/2024 13:27, Jan Beulich wrote:
> >>>>>> In preparation of dropping the register parameters from
> >>>>>> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
> >>>>>> register state needs making available another way for the few key
> >>>>>> handlers which need it. Fake IRQ-like state.
> >>>>>>
> >>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>>>>> ---
> >>>>>> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
> >>>>>> other console poll functions we have, and it's unclear whether that's
> >>>>>> actually generally correct.
> >>>>>
> >>>>> Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
> >>>>> run_in_exception() doesn't exist. But looking at the caller, no-on seems 
> >>>>> to care about the 'regs'. So is this just a latent bug?
> >>>>
> >>>> What do you mean by "doesn't exist"? ns16550_poll() assumes it exists.
> >>>> And I can spot any use of guest_user_regs() on the respective generic
> >>>> or Arm-specific bug.c paths.
> >>>>
> >>>>> BTW, do you have an idea why the poll function is not run in an 
> >>>>> exception handler?
> >>>>
> >>>> "The poll function" being which one? If you mean the one in xhci-dbc.c
> >>>> then that's why I had Cc-ed Marek. Moving him to To: - maybe that
> >>>> manages to finally catch his attention.
> >>>
> >>> TBH, I don't know. That's part of the original xue patch at
> >>> https://github.com/connojd/xue/blob/master/patches/xen-xue-dbgp.patch
> >>> and it works for me as it is.
> >>
> >> "Works" meaning what? Doesn't crash on you? Or does also provide
> >> sensible output in _all_ cases (i.e. including when e.g. the poll
> >> happens to run on an idle vCPU)?
> > 
> > Generally provides sensible output, for example during boot (it is using
> > idle vCPU then, right?).
> 
> Before Dom0 is started: Yes. With the exception of the phase where PV
> Dom0's page tables are constructed, albeit in that time window
> guest_cpu_user_regs() shouldn't yield sensible data either. I can only
> say I'm surprised; since I have no way to properly test with an XHCI
> debug port, I'd have to see about faking something to convince myself
> (unless you were to supply example output).

Would you like me to test this series with xhci console? Or maybe add
some extra debug prints and include their output? But note, printk from
inside console code generally leads to deadlocks. What I did for some
debugging was to log into some separate buffer and dump it later.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Jan Beulich 6 months, 3 weeks ago
On 13.02.2024 16:11, Marek Marczykowski wrote:
> On Tue, Feb 13, 2024 at 04:00:32PM +0100, Jan Beulich wrote:
>> On 13.02.2024 15:53, Marek Marczykowski wrote:
>>> On Tue, Feb 13, 2024 at 08:45:54AM +0100, Jan Beulich wrote:
>>>> On 13.02.2024 04:43, Marek Marczykowski wrote:
>>>>> On Mon, Feb 12, 2024 at 10:04:38AM +0100, Jan Beulich wrote:
>>>>>> On 08.02.2024 23:00, Julien Grall wrote:
>>>>>>> On 05/02/2024 13:27, Jan Beulich wrote:
>>>>>>>> In preparation of dropping the register parameters from
>>>>>>>> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
>>>>>>>> register state needs making available another way for the few key
>>>>>>>> handlers which need it. Fake IRQ-like state.
>>>>>>>>
>>>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>>>>> ---
>>>>>>>> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
>>>>>>>> other console poll functions we have, and it's unclear whether that's
>>>>>>>> actually generally correct.
>>>>>>>
>>>>>>> Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
>>>>>>> run_in_exception() doesn't exist. But looking at the caller, no-on seems 
>>>>>>> to care about the 'regs'. So is this just a latent bug?
>>>>>>
>>>>>> What do you mean by "doesn't exist"? ns16550_poll() assumes it exists.
>>>>>> And I can spot any use of guest_user_regs() on the respective generic
>>>>>> or Arm-specific bug.c paths.
>>>>>>
>>>>>>> BTW, do you have an idea why the poll function is not run in an 
>>>>>>> exception handler?
>>>>>>
>>>>>> "The poll function" being which one? If you mean the one in xhci-dbc.c
>>>>>> then that's why I had Cc-ed Marek. Moving him to To: - maybe that
>>>>>> manages to finally catch his attention.
>>>>>
>>>>> TBH, I don't know. That's part of the original xue patch at
>>>>> https://github.com/connojd/xue/blob/master/patches/xen-xue-dbgp.patch
>>>>> and it works for me as it is.
>>>>
>>>> "Works" meaning what? Doesn't crash on you? Or does also provide
>>>> sensible output in _all_ cases (i.e. including when e.g. the poll
>>>> happens to run on an idle vCPU)?
>>>
>>> Generally provides sensible output, for example during boot (it is using
>>> idle vCPU then, right?).
>>
>> Before Dom0 is started: Yes. With the exception of the phase where PV
>> Dom0's page tables are constructed, albeit in that time window
>> guest_cpu_user_regs() shouldn't yield sensible data either. I can only
>> say I'm surprised; since I have no way to properly test with an XHCI
>> debug port, I'd have to see about faking something to convince myself
>> (unless you were to supply example output).
> 
> Would you like me to test this series with xhci console?

The behavior shouldn't really be connected to this series. But yes, 'd'
debug key output (just the part for the CPU the key handling was
actually invoked from) with the xhci debug console would be of
interest, for the case where that CPU at that time runs an idle vCPU.

> Or maybe add
> some extra debug prints and include their output? But note, printk from
> inside console code generally leads to deadlocks. What I did for some
> debugging was to log into some separate buffer and dump it later.

Right, this would be more involved.

Jan
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Marek Marczykowski 6 months, 3 weeks ago
On Tue, Feb 13, 2024 at 04:44:04PM +0100, Jan Beulich wrote:
> On 13.02.2024 16:11, Marek Marczykowski wrote:
> > On Tue, Feb 13, 2024 at 04:00:32PM +0100, Jan Beulich wrote:
> >> On 13.02.2024 15:53, Marek Marczykowski wrote:
> >>> On Tue, Feb 13, 2024 at 08:45:54AM +0100, Jan Beulich wrote:
> >>>> On 13.02.2024 04:43, Marek Marczykowski wrote:
> >>>>> On Mon, Feb 12, 2024 at 10:04:38AM +0100, Jan Beulich wrote:
> >>>>>> On 08.02.2024 23:00, Julien Grall wrote:
> >>>>>>> On 05/02/2024 13:27, Jan Beulich wrote:
> >>>>>>>> In preparation of dropping the register parameters from
> >>>>>>>> serial_[rt]x_interrupt() and in turn from IRQ handler functions,
> >>>>>>>> register state needs making available another way for the few key
> >>>>>>>> handlers which need it. Fake IRQ-like state.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>>>>>>> ---
> >>>>>>>> The use of guest_cpu_user_regs() in dbc_uart_poll() is inconsistent with
> >>>>>>>> other console poll functions we have, and it's unclear whether that's
> >>>>>>>> actually generally correct.
> >>>>>>>
> >>>>>>> Is it? Looking at ns16550_poll() we would pass guest_user_regs() if 
> >>>>>>> run_in_exception() doesn't exist. But looking at the caller, no-on seems 
> >>>>>>> to care about the 'regs'. So is this just a latent bug?
> >>>>>>
> >>>>>> What do you mean by "doesn't exist"? ns16550_poll() assumes it exists.
> >>>>>> And I can spot any use of guest_user_regs() on the respective generic
> >>>>>> or Arm-specific bug.c paths.
> >>>>>>
> >>>>>>> BTW, do you have an idea why the poll function is not run in an 
> >>>>>>> exception handler?
> >>>>>>
> >>>>>> "The poll function" being which one? If you mean the one in xhci-dbc.c
> >>>>>> then that's why I had Cc-ed Marek. Moving him to To: - maybe that
> >>>>>> manages to finally catch his attention.
> >>>>>
> >>>>> TBH, I don't know. That's part of the original xue patch at
> >>>>> https://github.com/connojd/xue/blob/master/patches/xen-xue-dbgp.patch
> >>>>> and it works for me as it is.
> >>>>
> >>>> "Works" meaning what? Doesn't crash on you? Or does also provide
> >>>> sensible output in _all_ cases (i.e. including when e.g. the poll
> >>>> happens to run on an idle vCPU)?
> >>>
> >>> Generally provides sensible output, for example during boot (it is using
> >>> idle vCPU then, right?).
> >>
> >> Before Dom0 is started: Yes. With the exception of the phase where PV
> >> Dom0's page tables are constructed, albeit in that time window
> >> guest_cpu_user_regs() shouldn't yield sensible data either. I can only
> >> say I'm surprised; since I have no way to properly test with an XHCI
> >> debug port, I'd have to see about faking something to convince myself
> >> (unless you were to supply example output).
> > 
> > Would you like me to test this series with xhci console?
> 
> The behavior shouldn't really be connected to this series. But yes, 'd'
> debug key output (just the part for the CPU the key handling was
> actually invoked from) with the xhci debug console would be of
> interest, for the case where that CPU at that time runs an idle vCPU.

I managed to press 'd' before dom0 started. Full output at
https://gist.github.com/marmarek/a495cd666f4aafed3a5cfcb8393f515b (it's
Alder Lake, and smt=off, so CPU numbering is weird).
Interestingly, I do _not_ see output for CPU0, where I'd expect the
key handler to run... I see all the idle ones, plus one doing memory
scrubbing.
But also, I don't see info about the handling CPU when doing `xl
debug-key d`. At one time, with `xl debug-key d` I got this:

(XEN) *** Dumping CPU6 guest state (d0v7): ***
(XEN) ----[ Xen-4.18-unstable  x86_64  debug=y  Tainted:  M     ]----
(XEN) CPU:    6
(XEN) RIP:    e033:[<ffffffff81e1546a>]
(XEN) RFLAGS: 0000000000000286   EM: 0   CONTEXT: pv guest (d0v7)
(XEN) rax: 0000000000000023   rbx: 0000000000000005   rcx: ffffffff81e1546a
(XEN) rdx: 0000000000000000   rsi: 0000000000000000   rdi: 000079147611e010
(XEN) rbp: ffff88810db53200   rsp: ffffc90041c6bde0   r8:  0000000000000000
(XEN) r9:  0000000000000000   r10: 0000000000000000   r11: 0000000000000286
(XEN) r12: 0000000000305000   r13: 00007ffc61097f40   r14: ffff88810db53200
(XEN) r15: 0000000000000000   cr0: 0000000080050033   cr4: 0000000000b526e0
(XEN) cr3: 00000004ae2a7000   cr2: 00000000006d3118
(XEN) fsb: 0000791475b8a380   gsb: ffff8881897c0000   gss: 0000000000000000
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e02b   cs: e033
(XEN) Guest stack trace from rsp=ffffc90041c6bde0:
(XEN)    0000000000000001 0000000000000000 ffffffffc02905a6 0000000000000023
(XEN)    000079147611e010 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 8064129fc747f100 ffffffffc0291568 0000000000305000
(XEN)    8064129fc747f100 0000000000000005 ffffffff813f7d4d ffffc90041c6bf58
(XEN)    ffffc90041c6bf48 0000000000000000 0000000000000000 0000000000000000
(XEN)    ffffffff81e16158 00000000006d3118 ffffc90041c6bf58 0000000000000040
(XEN)    ffffffff8132f6bb 0000000000000006 ffffc90041c6bf58 00000000006d3118
(XEN)    0000000000000255 ffff888102cf8880 ffff888102cf88f0 ffffffff8108746f
(XEN)    0000000000000000 0000000000000002 ffffc90041c6bf58 ffffc90041c6bf58
(XEN)    00000000006d3118 0000000000000000 0000000000000006 0000000000000000
(XEN)    0000000000000000 ffffffff81e1a975 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 ffffffff8200009b 000000000043d9b0
(XEN)    000000000043d990 00007ffc61097f90 00007ffc61097fc0 00007ffc61099d16
(XEN)    00000000006cab40 0000000000000246 0000000000000001 0000000000000000
(XEN)    0000000000000006 ffffffffffffffda 0000791475f1ed6f 00007ffc61097f40
(XEN)    0000000000305000 0000000000000005 0000000000000010 0000791475f1ed6f
(XEN)    0000000000000033 0000000000000246 00007ffc61097ed0 000000000000002b
(XEN)     Fault while accessing guest memory.
(XEN) 
(XEN) *** Dumping CPU0 host state: ***
(XEN) ----[ Xen-4.18-unstable  x86_64  debug=y  Tainted:  M     ]----
(XEN) CPU:    0
(XEN) RIP:    e008:[<ffff82d04022c07e>] _spin_unlock_irqrestore+0x21/0x27
(XEN) RFLAGS: 0000000000000206   CONTEXT: hypervisor
(XEN) rax: ffff82d0405c1038   rbx: 0000000000000200   rcx: 0000000000000008
(XEN) rdx: ffff830856d07fff   rsi: ffff8308529d5b28   rdi: ffff8308529d5b20
(XEN) rbp: ffff830856d07dc8   rsp: ffff830856d07dc0   r8:  0000000000000001
(XEN) r9:  ffff8308529d5b20   r10: ffff82d0405c13a0   r11: 000000d091e62221
(XEN) r12: ffff82d040476898   r13: 0000000000000296   r14: ffff82d040476918
(XEN) r15: ffff82cffff04700   cr0: 0000000080050033   cr4: 0000000000b526e0
(XEN) cr3: 000000082e7ff000   cr2: ffff888109538618
(XEN) fsb: 0000000000000000   gsb: ffff888189600000   gss: 0000000000000000
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
(XEN) Xen code around <ffff82d04022c07e> (_spin_unlock_irqrestore+0x21/0x27):
(XEN)  fd ff ff 48 09 1c 24 9d <48> 8b 5d f8 c9 c3 0f b7 47 04 66 25 ff 0f 66 3d
(XEN) Xen stack trace from rsp=ffff830856d07dc0:
(XEN)    ffff82d0405d0c80 ffff830856d07e08 ffff82d040257c3f 0000000040476898
(XEN)    ffff82d0405c1280 ffff82d040257bca ffff82d040476898 000000d0911fcbc4
(XEN)    0000000000000000 ffff830856d07e30 ffff82d04022d55c ffff82d0405c1280
(XEN)    ffff8308529d5f00 ffff82d0405d0d68 ffff830856d07e70 ffff82d04022de59
(XEN)    ffff830856d07ef8 ffff82d0405c7f00 ffffffffffffffff ffff82d0405c7f00
(XEN)    ffff830856d07fff 0000000000000000 ffff830856d07ea8 ffff82d04022b53e
(XEN)    0000000000000000 0000000000007fff ffff82d0405c7f00 ffff82d0405c11d0
(XEN)    ffff82d0405db2a0 ffff830856d07eb8 ffff82d04022b5d1 ffff830856d07ef0
(XEN)    ffff82d0402fcd15 ffff82d0402fcc88 ffff8308528cb000 ffff830856d07ef8
(XEN)    ffff830856ce2000 0000000000000000 ffff830856d07e18 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 ffffffff82c1aa40
(XEN)    0000000000000000 0000000000000246 0000000000007ff0 0000000000000000
(XEN)    000000000fd109eb 0000000000000000 ffffffff81e153aa 4000000000000000
(XEN)    deadbeefdeadf00d deadbeefdeadf00d 0000010000000000 ffffffff81e153aa
(XEN)    000000000000e033 0000000000000246 ffffffff82c03dd0 000000000000e02b
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000e01000000000 ffff830856ce1000 0000000000000000 0000000000b526e0
(XEN)    0000000000000000 0000000000000000 0006030300000000 0000000000000000
(XEN) Xen call trace:
(XEN)    [<ffff82d04022c07e>] R _spin_unlock_irqrestore+0x21/0x27
(XEN)    [<ffff82d040257c3f>] F xhci-dbc.c#dbc_uart_poll+0x75/0x17c
(XEN)    [<ffff82d04022d55c>] F timer.c#execute_timer+0x45/0x5c
(XEN)    [<ffff82d04022de59>] F timer.c#timer_softirq_action+0x71/0x275
(XEN)    [<ffff82d04022b53e>] F softirq.c#__do_softirq+0x94/0xbe
(XEN)    [<ffff82d04022b5d1>] F do_softirq+0x13/0x15
(XEN)    [<ffff82d0402fcd15>] F domain.c#idle_loop+0x8d/0xe6

(other CPUs in mwait-idle)

> > Or maybe add
> > some extra debug prints and include their output? But note, printk from
> > inside console code generally leads to deadlocks. What I did for some
> > debugging was to log into some separate buffer and dump it later.
> 
> Right, this would be more involved.
> 
> Jan

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Jan Beulich 6 months, 3 weeks ago
On 15.02.2024 03:19, Marek Marczykowski wrote:
> On Tue, Feb 13, 2024 at 04:44:04PM +0100, Jan Beulich wrote:
>> On 13.02.2024 16:11, Marek Marczykowski wrote:
>>> On Tue, Feb 13, 2024 at 04:00:32PM +0100, Jan Beulich wrote:
>>>> On 13.02.2024 15:53, Marek Marczykowski wrote:
>>>>> Generally provides sensible output, for example during boot (it is using
>>>>> idle vCPU then, right?).
>>>>
>>>> Before Dom0 is started: Yes. With the exception of the phase where PV
>>>> Dom0's page tables are constructed, albeit in that time window
>>>> guest_cpu_user_regs() shouldn't yield sensible data either. I can only
>>>> say I'm surprised; since I have no way to properly test with an XHCI
>>>> debug port, I'd have to see about faking something to convince myself
>>>> (unless you were to supply example output).
>>>
>>> Would you like me to test this series with xhci console?
>>
>> The behavior shouldn't really be connected to this series. But yes, 'd'
>> debug key output (just the part for the CPU the key handling was
>> actually invoked from) with the xhci debug console would be of
>> interest, for the case where that CPU at that time runs an idle vCPU.
> 
> I managed to press 'd' before dom0 started. Full output at
> https://gist.github.com/marmarek/a495cd666f4aafed3a5cfcb8393f515b (it's
> Alder Lake, and smt=off, so CPU numbering is weird).
> Interestingly, I do _not_ see output for CPU0, where I'd expect the
> key handler to run... I see all the idle ones, plus one doing memory
> scrubbing.

Which is precisely the problem, just in not exactly the manifestation
I expected. In dump_execstate() we dump host state only if the
incoming regs don't indicate guest state. Yet for the idle vCPU they
(wrongly) do here - see how guest_mode() calculates the delta to what
guest_cpu_user_regs() returns, i.e. 0 when what guest_cpu_user_regs()
returned is passed in.

Guest state dumping is suppressed for idle vCPU-s. Hence no output
at all for the CPU where the key processing was actually invoked
from.

> But also, I don't see info about the handling CPU when doing `xl
> debug-key d`.

I'm afraid I'm confused, ...

> At one time, with `xl debug-key d` I got this:
> 
> (XEN) *** Dumping CPU6 guest state (d0v7): ***
> (XEN) ----[ Xen-4.18-unstable  x86_64  debug=y  Tainted:  M     ]----
> (XEN) CPU:    6
> (XEN) RIP:    e033:[<ffffffff81e1546a>]
> (XEN) RFLAGS: 0000000000000286   EM: 0   CONTEXT: pv guest (d0v7)
> (XEN) rax: 0000000000000023   rbx: 0000000000000005   rcx: ffffffff81e1546a
> (XEN) rdx: 0000000000000000   rsi: 0000000000000000   rdi: 000079147611e010
> (XEN) rbp: ffff88810db53200   rsp: ffffc90041c6bde0   r8:  0000000000000000
> (XEN) r9:  0000000000000000   r10: 0000000000000000   r11: 0000000000000286
> (XEN) r12: 0000000000305000   r13: 00007ffc61097f40   r14: ffff88810db53200
> (XEN) r15: 0000000000000000   cr0: 0000000080050033   cr4: 0000000000b526e0
> (XEN) cr3: 00000004ae2a7000   cr2: 00000000006d3118
> (XEN) fsb: 0000791475b8a380   gsb: ffff8881897c0000   gss: 0000000000000000
> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e02b   cs: e033
> (XEN) Guest stack trace from rsp=ffffc90041c6bde0:
> (XEN)    0000000000000001 0000000000000000 ffffffffc02905a6 0000000000000023
> (XEN)    000079147611e010 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000000000000000 8064129fc747f100 ffffffffc0291568 0000000000305000
> (XEN)    8064129fc747f100 0000000000000005 ffffffff813f7d4d ffffc90041c6bf58
> (XEN)    ffffc90041c6bf48 0000000000000000 0000000000000000 0000000000000000
> (XEN)    ffffffff81e16158 00000000006d3118 ffffc90041c6bf58 0000000000000040
> (XEN)    ffffffff8132f6bb 0000000000000006 ffffc90041c6bf58 00000000006d3118
> (XEN)    0000000000000255 ffff888102cf8880 ffff888102cf88f0 ffffffff8108746f
> (XEN)    0000000000000000 0000000000000002 ffffc90041c6bf58 ffffc90041c6bf58
> (XEN)    00000000006d3118 0000000000000000 0000000000000006 0000000000000000
> (XEN)    0000000000000000 ffffffff81e1a975 0000000000000000 0000000000000000
> (XEN)    0000000000000000 0000000000000000 ffffffff8200009b 000000000043d9b0
> (XEN)    000000000043d990 00007ffc61097f90 00007ffc61097fc0 00007ffc61099d16
> (XEN)    00000000006cab40 0000000000000246 0000000000000001 0000000000000000
> (XEN)    0000000000000006 ffffffffffffffda 0000791475f1ed6f 00007ffc61097f40
> (XEN)    0000000000305000 0000000000000005 0000000000000010 0000791475f1ed6f
> (XEN)    0000000000000033 0000000000000246 00007ffc61097ed0 000000000000002b
> (XEN)     Fault while accessing guest memory.
> (XEN) 
> (XEN) *** Dumping CPU0 host state: ***
> (XEN) ----[ Xen-4.18-unstable  x86_64  debug=y  Tainted:  M     ]----
> (XEN) CPU:    0
> (XEN) RIP:    e008:[<ffff82d04022c07e>] _spin_unlock_irqrestore+0x21/0x27
> (XEN) RFLAGS: 0000000000000206   CONTEXT: hypervisor
> (XEN) rax: ffff82d0405c1038   rbx: 0000000000000200   rcx: 0000000000000008
> (XEN) rdx: ffff830856d07fff   rsi: ffff8308529d5b28   rdi: ffff8308529d5b20
> (XEN) rbp: ffff830856d07dc8   rsp: ffff830856d07dc0   r8:  0000000000000001
> (XEN) r9:  ffff8308529d5b20   r10: ffff82d0405c13a0   r11: 000000d091e62221
> (XEN) r12: ffff82d040476898   r13: 0000000000000296   r14: ffff82d040476918
> (XEN) r15: ffff82cffff04700   cr0: 0000000080050033   cr4: 0000000000b526e0
> (XEN) cr3: 000000082e7ff000   cr2: ffff888109538618
> (XEN) fsb: 0000000000000000   gsb: ffff888189600000   gss: 0000000000000000
> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
> (XEN) Xen code around <ffff82d04022c07e> (_spin_unlock_irqrestore+0x21/0x27):
> (XEN)  fd ff ff 48 09 1c 24 9d <48> 8b 5d f8 c9 c3 0f b7 47 04 66 25 ff 0f 66 3d
> (XEN) Xen stack trace from rsp=ffff830856d07dc0:
> (XEN)    ffff82d0405d0c80 ffff830856d07e08 ffff82d040257c3f 0000000040476898
> (XEN)    ffff82d0405c1280 ffff82d040257bca ffff82d040476898 000000d0911fcbc4
> (XEN)    0000000000000000 ffff830856d07e30 ffff82d04022d55c ffff82d0405c1280
> (XEN)    ffff8308529d5f00 ffff82d0405d0d68 ffff830856d07e70 ffff82d04022de59
> (XEN)    ffff830856d07ef8 ffff82d0405c7f00 ffffffffffffffff ffff82d0405c7f00
> (XEN)    ffff830856d07fff 0000000000000000 ffff830856d07ea8 ffff82d04022b53e
> (XEN)    0000000000000000 0000000000007fff ffff82d0405c7f00 ffff82d0405c11d0
> (XEN)    ffff82d0405db2a0 ffff830856d07eb8 ffff82d04022b5d1 ffff830856d07ef0
> (XEN)    ffff82d0402fcd15 ffff82d0402fcc88 ffff8308528cb000 ffff830856d07ef8
> (XEN)    ffff830856ce2000 0000000000000000 ffff830856d07e18 0000000000000000
> (XEN)    0000000000000000 0000000000000000 0000000000000000 ffffffff82c1aa40
> (XEN)    0000000000000000 0000000000000246 0000000000007ff0 0000000000000000
> (XEN)    000000000fd109eb 0000000000000000 ffffffff81e153aa 4000000000000000
> (XEN)    deadbeefdeadf00d deadbeefdeadf00d 0000010000000000 ffffffff81e153aa
> (XEN)    000000000000e033 0000000000000246 ffffffff82c03dd0 000000000000e02b
> (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN)    0000e01000000000 ffff830856ce1000 0000000000000000 0000000000b526e0
> (XEN)    0000000000000000 0000000000000000 0006030300000000 0000000000000000
> (XEN) Xen call trace:
> (XEN)    [<ffff82d04022c07e>] R _spin_unlock_irqrestore+0x21/0x27
> (XEN)    [<ffff82d040257c3f>] F xhci-dbc.c#dbc_uart_poll+0x75/0x17c
> (XEN)    [<ffff82d04022d55c>] F timer.c#execute_timer+0x45/0x5c
> (XEN)    [<ffff82d04022de59>] F timer.c#timer_softirq_action+0x71/0x275
> (XEN)    [<ffff82d04022b53e>] F softirq.c#__do_softirq+0x94/0xbe
> (XEN)    [<ffff82d04022b5d1>] F do_softirq+0x13/0x15
> (XEN)    [<ffff82d0402fcd15>] F domain.c#idle_loop+0x8d/0xe6

... this looks to be the issuing CPU. What I don't understand is why we
are in _spin_unlock_irqrestore() here, called out of dbc_uart_poll().

Btw - was any/all of this with or without the series here applied?

Jan
Re: [PATCH v3 1/8] serial: fake IRQ-regs context in poll handlers
Posted by Marek Marczykowski 6 months, 3 weeks ago
On Thu, Feb 15, 2024 at 09:39:41AM +0100, Jan Beulich wrote:
> On 15.02.2024 03:19, Marek Marczykowski wrote:
> > On Tue, Feb 13, 2024 at 04:44:04PM +0100, Jan Beulich wrote:
> >> On 13.02.2024 16:11, Marek Marczykowski wrote:
> >>> On Tue, Feb 13, 2024 at 04:00:32PM +0100, Jan Beulich wrote:
> >>>> On 13.02.2024 15:53, Marek Marczykowski wrote:
> >>>>> Generally provides sensible output, for example during boot (it is using
> >>>>> idle vCPU then, right?).
> >>>>
> >>>> Before Dom0 is started: Yes. With the exception of the phase where PV
> >>>> Dom0's page tables are constructed, albeit in that time window
> >>>> guest_cpu_user_regs() shouldn't yield sensible data either. I can only
> >>>> say I'm surprised; since I have no way to properly test with an XHCI
> >>>> debug port, I'd have to see about faking something to convince myself
> >>>> (unless you were to supply example output).
> >>>
> >>> Would you like me to test this series with xhci console?
> >>
> >> The behavior shouldn't really be connected to this series. But yes, 'd'
> >> debug key output (just the part for the CPU the key handling was
> >> actually invoked from) with the xhci debug console would be of
> >> interest, for the case where that CPU at that time runs an idle vCPU.
> > 
> > I managed to press 'd' before dom0 started. Full output at
> > https://gist.github.com/marmarek/a495cd666f4aafed3a5cfcb8393f515b (it's
> > Alder Lake, and smt=off, so CPU numbering is weird).
> > Interestingly, I do _not_ see output for CPU0, where I'd expect the
> > key handler to run... I see all the idle ones, plus one doing memory
> > scrubbing.
> 
> Which is precisely the problem, just in not exactly the manifestation
> I expected. In dump_execstate() we dump host state only if the
> incoming regs don't indicate guest state. Yet for the idle vCPU they
> (wrongly) do here - see how guest_mode() calculates the delta to what
> guest_cpu_user_regs() returns, i.e. 0 when what guest_cpu_user_regs()
> returned is passed in.
> 
> Guest state dumping is suppressed for idle vCPU-s. Hence no output
> at all for the CPU where the key processing was actually invoked
> from.
> 
> > But also, I don't see info about the handling CPU when doing `xl
> > debug-key d`.
> 
> I'm afraid I'm confused, ...
> 
> > At one time, with `xl debug-key d` I got this:
> > 
> > (XEN) *** Dumping CPU6 guest state (d0v7): ***
> > (XEN) ----[ Xen-4.18-unstable  x86_64  debug=y  Tainted:  M     ]----
> > (XEN) CPU:    6
> > (XEN) RIP:    e033:[<ffffffff81e1546a>]
> > (XEN) RFLAGS: 0000000000000286   EM: 0   CONTEXT: pv guest (d0v7)
> > (XEN) rax: 0000000000000023   rbx: 0000000000000005   rcx: ffffffff81e1546a
> > (XEN) rdx: 0000000000000000   rsi: 0000000000000000   rdi: 000079147611e010
> > (XEN) rbp: ffff88810db53200   rsp: ffffc90041c6bde0   r8:  0000000000000000
> > (XEN) r9:  0000000000000000   r10: 0000000000000000   r11: 0000000000000286
> > (XEN) r12: 0000000000305000   r13: 00007ffc61097f40   r14: ffff88810db53200
> > (XEN) r15: 0000000000000000   cr0: 0000000080050033   cr4: 0000000000b526e0
> > (XEN) cr3: 00000004ae2a7000   cr2: 00000000006d3118
> > (XEN) fsb: 0000791475b8a380   gsb: ffff8881897c0000   gss: 0000000000000000
> > (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e02b   cs: e033
> > (XEN) Guest stack trace from rsp=ffffc90041c6bde0:
> > (XEN)    0000000000000001 0000000000000000 ffffffffc02905a6 0000000000000023
> > (XEN)    000079147611e010 0000000000000000 0000000000000000 0000000000000000
> > (XEN)    0000000000000000 8064129fc747f100 ffffffffc0291568 0000000000305000
> > (XEN)    8064129fc747f100 0000000000000005 ffffffff813f7d4d ffffc90041c6bf58
> > (XEN)    ffffc90041c6bf48 0000000000000000 0000000000000000 0000000000000000
> > (XEN)    ffffffff81e16158 00000000006d3118 ffffc90041c6bf58 0000000000000040
> > (XEN)    ffffffff8132f6bb 0000000000000006 ffffc90041c6bf58 00000000006d3118
> > (XEN)    0000000000000255 ffff888102cf8880 ffff888102cf88f0 ffffffff8108746f
> > (XEN)    0000000000000000 0000000000000002 ffffc90041c6bf58 ffffc90041c6bf58
> > (XEN)    00000000006d3118 0000000000000000 0000000000000006 0000000000000000
> > (XEN)    0000000000000000 ffffffff81e1a975 0000000000000000 0000000000000000
> > (XEN)    0000000000000000 0000000000000000 ffffffff8200009b 000000000043d9b0
> > (XEN)    000000000043d990 00007ffc61097f90 00007ffc61097fc0 00007ffc61099d16
> > (XEN)    00000000006cab40 0000000000000246 0000000000000001 0000000000000000
> > (XEN)    0000000000000006 ffffffffffffffda 0000791475f1ed6f 00007ffc61097f40
> > (XEN)    0000000000305000 0000000000000005 0000000000000010 0000791475f1ed6f
> > (XEN)    0000000000000033 0000000000000246 00007ffc61097ed0 000000000000002b
> > (XEN)     Fault while accessing guest memory.
> > (XEN) 
> > (XEN) *** Dumping CPU0 host state: ***
> > (XEN) ----[ Xen-4.18-unstable  x86_64  debug=y  Tainted:  M     ]----
> > (XEN) CPU:    0
> > (XEN) RIP:    e008:[<ffff82d04022c07e>] _spin_unlock_irqrestore+0x21/0x27
> > (XEN) RFLAGS: 0000000000000206   CONTEXT: hypervisor
> > (XEN) rax: ffff82d0405c1038   rbx: 0000000000000200   rcx: 0000000000000008
> > (XEN) rdx: ffff830856d07fff   rsi: ffff8308529d5b28   rdi: ffff8308529d5b20
> > (XEN) rbp: ffff830856d07dc8   rsp: ffff830856d07dc0   r8:  0000000000000001
> > (XEN) r9:  ffff8308529d5b20   r10: ffff82d0405c13a0   r11: 000000d091e62221
> > (XEN) r12: ffff82d040476898   r13: 0000000000000296   r14: ffff82d040476918
> > (XEN) r15: ffff82cffff04700   cr0: 0000000080050033   cr4: 0000000000b526e0
> > (XEN) cr3: 000000082e7ff000   cr2: ffff888109538618
> > (XEN) fsb: 0000000000000000   gsb: ffff888189600000   gss: 0000000000000000
> > (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
> > (XEN) Xen code around <ffff82d04022c07e> (_spin_unlock_irqrestore+0x21/0x27):
> > (XEN)  fd ff ff 48 09 1c 24 9d <48> 8b 5d f8 c9 c3 0f b7 47 04 66 25 ff 0f 66 3d
> > (XEN) Xen stack trace from rsp=ffff830856d07dc0:
> > (XEN)    ffff82d0405d0c80 ffff830856d07e08 ffff82d040257c3f 0000000040476898
> > (XEN)    ffff82d0405c1280 ffff82d040257bca ffff82d040476898 000000d0911fcbc4
> > (XEN)    0000000000000000 ffff830856d07e30 ffff82d04022d55c ffff82d0405c1280
> > (XEN)    ffff8308529d5f00 ffff82d0405d0d68 ffff830856d07e70 ffff82d04022de59
> > (XEN)    ffff830856d07ef8 ffff82d0405c7f00 ffffffffffffffff ffff82d0405c7f00
> > (XEN)    ffff830856d07fff 0000000000000000 ffff830856d07ea8 ffff82d04022b53e
> > (XEN)    0000000000000000 0000000000007fff ffff82d0405c7f00 ffff82d0405c11d0
> > (XEN)    ffff82d0405db2a0 ffff830856d07eb8 ffff82d04022b5d1 ffff830856d07ef0
> > (XEN)    ffff82d0402fcd15 ffff82d0402fcc88 ffff8308528cb000 ffff830856d07ef8
> > (XEN)    ffff830856ce2000 0000000000000000 ffff830856d07e18 0000000000000000
> > (XEN)    0000000000000000 0000000000000000 0000000000000000 ffffffff82c1aa40
> > (XEN)    0000000000000000 0000000000000246 0000000000007ff0 0000000000000000
> > (XEN)    000000000fd109eb 0000000000000000 ffffffff81e153aa 4000000000000000
> > (XEN)    deadbeefdeadf00d deadbeefdeadf00d 0000010000000000 ffffffff81e153aa
> > (XEN)    000000000000e033 0000000000000246 ffffffff82c03dd0 000000000000e02b
> > (XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
> > (XEN)    0000e01000000000 ffff830856ce1000 0000000000000000 0000000000b526e0
> > (XEN)    0000000000000000 0000000000000000 0006030300000000 0000000000000000
> > (XEN) Xen call trace:
> > (XEN)    [<ffff82d04022c07e>] R _spin_unlock_irqrestore+0x21/0x27
> > (XEN)    [<ffff82d040257c3f>] F xhci-dbc.c#dbc_uart_poll+0x75/0x17c
> > (XEN)    [<ffff82d04022d55c>] F timer.c#execute_timer+0x45/0x5c
> > (XEN)    [<ffff82d04022de59>] F timer.c#timer_softirq_action+0x71/0x275
> > (XEN)    [<ffff82d04022b53e>] F softirq.c#__do_softirq+0x94/0xbe
> > (XEN)    [<ffff82d04022b5d1>] F do_softirq+0x13/0x15
> > (XEN)    [<ffff82d0402fcd15>] F domain.c#idle_loop+0x8d/0xe6
> 
> ... this looks to be the issuing CPU. What I don't understand is why we
> are in _spin_unlock_irqrestore() here, called out of dbc_uart_poll().
> 
> Btw - was any/all of this with or without the series here applied?

This was without this series applied.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab