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

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxx>
>>>>>>>> ---
>>>>>>>> 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