Re: [PATCH v3 01/10] drivers/char: Add support for USB3 DbC debugger

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.08.2022 16:21, Jan Beulich wrote:
> On 04.08.2022 15:43, Marek Marczykowski-Górecki wrote:
>> On Thu, Aug 04, 2022 at 02:57:49PM +0200, Jan Beulich wrote:
>>> On 26.07.2022 05:23, Marek Marczykowski-Górecki wrote:
>>>> +struct dbc {
>>>> +    struct dbc_reg __iomem *dbc_reg;
>>>> +    struct xhci_dbc_ctx *dbc_ctx;
>>>> +    struct xhci_erst_segment *dbc_erst;
>>>> +    struct xhci_trb_ring dbc_ering;
>>>> +    struct xhci_trb_ring dbc_oring;
>>>> +    struct xhci_trb_ring dbc_iring;
>>>> +    struct dbc_work_ring dbc_owork;
>>>> +    struct xhci_string_descriptor *dbc_str;
>>>
>>> I'm afraid I still don't see why the static page this field is being
>>> initialized with is necessary. Can't you have a static variable (of
>>> some struct type) all pre-filled at build time, which you then apply
>>> virt_to_maddr() to in order to fill the respective dbc_ctx fields?
>>
>> I need to keep this structure somewhere DMA-reachable for the device (as
>> in - included in appropriate IOMMU context). Patch 8/10 is doing it. And
>> also, patch 8/10 is putting it together with other DMA-reachable
>> structures (not a separate page on its own). If I'd make it a separate
>> static variable (not part of that later struct), I'd need to reserve the
>> whole page for it - to guarantee no unrelated data lives on the same
>> (DMA-reachable) page.
>>
>> As for statically initializing it, if would require the whole
>> (multi-page DMA-reachable) thing living in .data, not .bss, so a bigger
>> binary (not a huge concern due to compression, but still). But more
>> importantly, I don't know how to do it in a readable way, and you have
>> complained about readability of initializer of this structure in v2.
>>
>>> That struct will be quite a bit less than a page's worth in size.
>>
>> See above - it cannot share page with unrelated Xen data.
> 
> I have to admit that I'd see no issue if these lived side by side with
> e.g. other string literals. The more that the device is supposed to be
> exposed to Dom0 only anyway, and hence that'll be the only domain able
> to get at that data.

Actually: With your plan to expose the device to a DomU, how is that
going to work without tool stack adjustments? Wouldn't you need to
prevent in particular HVM/PVH guests from using the GFNs corresponding
to the MFNs where this Xen data is? The minimal requirement then would
seem to be an E820 reserved range for the area.

Jan