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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.08.2022 16:34, Jan Beulich wrote:
> 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.

I guess this was rubbish - by mimic-ing RMRRs or their AMD equivalents,
the tool stack ought to be taking care of this already.

Jan