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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.07.2022 11:27, Marek Marczykowski-Górecki wrote:
> On Mon, Jul 04, 2022 at 08:05:06AM +0200, Jan Beulich wrote:
>> 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.
> 
> Looking at it, I see another issue - Xen uses superpages, so I'd either
> need to:
>  - reserve the whole superpage to be able to release it later (waste 6
>    pages if xue is used), or
>  - shatter superpage when releasing unused xue buffers

That's part of the reason for my desire to generalize this.

> First one is probably less bad. But maybe, instead of doing all this,
> add xue to the menuconfig (make the prompt visible) with appropriate
> note about wasting 58 pages when built-in but not enabled. What do you
> think?

I don't mind it being done this way for now.

Jan