Re: [Xen-devel] [PATCH v2 0/2] ARM: ACPI: IORT: Hide SMMU from hardware domain's IORT table

[Manish Jaggi <mjaggi@xxxxxxxxxxxxxxxxxx>]

Hello Julien,

On 10/4/2017 12:12 AM, Julien Grall wrote:
> Hello,
>
> On 25/09/17 05:22, Manish Jaggi wrote:
> > On 9/22/2017 7:42 PM, Andre Przywara wrote:
> > > Hi Manish,
> > >
> > > On 11/09/17 22:33, mjaggi@xxxxxxxxxxxxxxxxxx wrote:
> > > > From: Manish Jaggi <mjaggi@xxxxxxxxxx>
> > > >
> > > > The set is divided into two patches. First one calculates the size 
> > > > of IORT
> > > > while second one writes the IORT table itself.
> > > It would be good if you could give a quick introduction *why* this set
> > > is needed here (and introduce IORT to the casual reader).
> > > In general some more high-level documentation on your functions 
> > > would be
> > > good, as it took me quite some time to understand what each function 
> > > does.
> > ok, will add more documentation.
> > > So my understanding is:
> > > phase 1:
> > > - go over each entry in each RC node
> > Rather than each entry (which could be a large number) I am taking 
> > the complete range and checking it with the same logic.
> > If the ID range is a subset or a super-set of id range in smmu, new 
> > id range is created.
> > So if pci_rc node has an id map 
> > {p_input-base,p_output-base,p_out_ref, p_count} and it an output 
> > reference to smmu node with id-map
> > {s_input-base, s_output-base,s_out_ref,  s_count}, based on the the 
> > the s_count and s_input/p_output the new id-map is created with 
> > {p_input, s_output, s_out_ref, adjusted_count}
> > update_id_mapping function does that.
> >
> > So I am following the same logic. We can chat over IRC / I can give a 
> > code walk-through ...
> > > -   if that points to an SMMU node, go over each outgoing ITS entry and
> > > find overlaps with this RC entry
> > > -     for each overlap create a new entry in a list with this RC
> > > pointing to the ITS directly
> > >
> > > phase 2, creating the new IORT
> > > - go over each RC node
> > > -   if that points to an ITS, copy through IORT entries
> > > -   if that points to an SMMU, replace with the remapped entries
> > > - go over each ITS node
> > > -   copy through IORT entries
> > Thats exactly what this patch does.
What are you comments on the current patch approach to hide smmu nodes.
I have answered to your comments, see below.
IMHO we can reuse most of the fixup code here.

> > > So I believe this would do the trick and you end up with an efficient
> > > representation of the IORT without SMMUs - at least for RC nodes.
> > >
> > > After some brainstorming with Julien we found two problems:
> > > 1) This only covers RC nodes, but not "named components" (platform
> > > devices), which we will need. That should be fixable by removing the
> > > hardcoded IORT node types in the code and treating NC nodes like RC 
> > > nodes.
> > Yes, so first we can take this as a base, once this is ok, I can add 
> > support for named components.
> > > 2) Eventually we will need *virtual* deviceID support, for DomUs. 
> > > Now we
> I am a bit surprised that you answered to the e-mail but didn't 
> provide any opinion on 2).
Apologies for that.
> > > could start introducing that already, also doing some virtual mapping
> > > for Dom0. The ITS code would then translate each virtual device ID that
> > > Dom0 requests into a hardware device ID.
> > > I agree that this means a lot more work, but we will need it anyway.
> I am a bit surprised that you answered to the e-mail but didn't 
> provide any opinion on 2).
Apologies for that. Sorry to surprise you twice :)

IMHO It was a bit obvious for DomU and I was waiting to hear what other 
would say on this.
as (2) below.
Moreover we need to discuss IORT generation for DomU
- could be done by xl tools
or xen should do it.

Also this is the part of PCI passthrough flow so that also might change 
few things.
But from pov of dom0 smmu hiding, it is a different flow and is coupled 
with PCI PT.
> > > I think 1) can be solved using this series as a base. I have quite some
> > > comments ready for the patches, shall we follow this route.
> > >
> > > 2) obviously would change the game completely. We need to sit down and
> > > design this properly. Probably this means that Xen parses the IORT and
> > > builds internal representations of the mappings, 
Can you please add more detail on the internal representations of the 
mappings.
IIUC the information is already there in ACPI tables, would it not add 
extra overhead of abstractions to maintain.
Enumeration of PCI devices would generate a pci list which would be 
anyways separate.
> > > which are consulted as
> > > needed when passing through devices. The guest's (that would include
> > > Dom0) IORT would then be generated completely from scratch.
I have a different opinion here, dom0 IORT would is most cases be very 
close to host IORT sans smmu nodes and few platform devices.
And which platform devices to hide would probably depend on the xen 
command line,
For instance for dom0 we would copy ITS information while for domU it 
would have to be generated, so scratch would be more for domU.
We could have a common code for creating IORT structure but it would be 
a bit complex code with lot of abstractions and callbacks, so I suggest 
that keeping code simpler would be better.
> > > I would like to hear your opinion on this. I will try to discuss the
> > > feasibility of 2) with people at Connect. It would be good if we could
> > > decide whether this is the way to go or we should use a solution based
> > > on this series.
> Andre, Stefano and I had a chat during connect about how we want to 
> see IORT support in Xen. In the near future, IORT will be used for 
> different components accross the hypervisor (ITS, SMMU...) and as a 
> way to
> communicate the topology to the guests.
Are we thinking about virtual SMMU here as well?
> The IORT for the hardware domain is just a specific case as it is 
> based pre-existing information. But because of removing nodes (e.g 
> SMMU nodes and probably the PMU nodes), it is basically a full re-write.
> So I would consider of full separate the logic of generating the IORT 
> table from the host IORT table. By that I mean not browsing the host 
> IORT when generating the host.
by "the host" you mean dom0 IORT  ?
> This has two benefits:
>     1) The code generation could be re-used for generating the guest 
> IORT later on.
See my comment above
>     2) See 2) from Andre
>     3) We could decide in a finer grain which devices (e.g platform 
> device) Dom0 can see.
ok,
> So, IHMO, we should take a different approach from this series and 
> extending the scope of it. Rather than focusing on only IORT for the 
> hardware, I would be better to see IORT as a whole. I.e  how IORT will 
> interact with the hypervisor?
> For instance, likely you would need to parse the IORT in quite a few 
> places in Xen. It would be better to get IORT parsed only once and 
> store the information in Xen like data-structures.
I am thinking of reusing much of ACPI tables for that and introducing 
less abstractions.
> This require more work than this current scope of this series. But I 
> think it would helful for future work such as PCI passthrough support.
> Any opinions?
for DomUs it is tied to PCI PT, so both design should evolve together.
> Cheers,

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel