Re: [PATCH v1 01/10] drivers/char: Add support for Xue USB3 debugger

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.07.2022 14:17, Marek Marczykowski-Górecki wrote:
> On Thu, Jun 23, 2022 at 11:29:31AM +0200, Jan Beulich wrote:
>> On 22.06.2022 17:47, Marek Marczykowski-Górecki wrote:
>>> On Wed, Jun 15, 2022 at 04:25:54PM +0200, Jan Beulich wrote:
>>>> On 07.06.2022 16:30, Marek Marczykowski-Górecki wrote:
>>>>> +    memset(xue, 0, sizeof(*xue));
>>>>> +
>>>>> +    xue->dbc_ctx = &ctx;
>>>>> +    xue->dbc_erst = &erst;
>>>>> +    xue->dbc_ering.trb = evt_trb;
>>>>> +    xue->dbc_oring.trb = out_trb;
>>>>> +    xue->dbc_iring.trb = in_trb;
>>>>> +    xue->dbc_owork.buf = wrk_buf;
>>>>> +    xue->dbc_str = str_buf;
>>>>
>>>> Especially the page-sized entities want allocating dynamically here, as
>>>> they won't be needed without the command line option requesting the use
>>>> of this driver.
>>>
>>> Are you okay with changing this only in patch 9, where I restructure those
>>> buffers anyway?
>>
>> I'm afraid I'll need to make it to patch 9 to answer this question. If
>> suitable dealt with later, I don't see a fundamental problem, as long
>> as it's clear then that I will request that this patch be committed in
>> a batch with that later one, not in isolation.
> 
> This turns out to be rather problematic. xue_uart_init() is called
> really early (as it should, to get console as early as possible). It's
> before even boot allocator is functioning, so I can't use alloc_boot_pages().
> Are there any other options for memory allocations at this point?

No "neat" ones. Stealing directly from E820 could be an option, but
would of course be heavily x86-centric.

> We are talking about 58 pages. It isn't much, but isn't exactly nothing
> either. Maybe there is way to keep the memory allocated statically as it
> is now, but return it to the allocator is xue is _not_ enabled?

Well, yes, treating them like .init.data would seem to be the least bad
alternative then, at least for now. Down the road we may want to generalize
what's needed here, as there are other full-page or larger memory areas
which are used only under certain conditions. We may e.g. want to introduce
an approach similar to Linux'es .brk section (recently renamed to .brk..bss
iirc). If a non-generalized approach ends up looking too ugly, I'd be okay
with keeping things as they're now, just with a respective justification
added to the patch description.

Jan