Re: [Xen-devel] [PATCH v3 04/14] AMD/IOMMU: use bit field for IRTE

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 19/07/2019 17:16, Jan Beulich wrote:
> On 19.07.2019 17:56, Andrew Cooper wrote:
>> On 16/07/2019 17:36, Jan Beulich wrote:
>>> At the same time restrict its scope to just the single source file
>>> actually using it, and abstract accesses by introducing a union of
>>> pointers. (A union of the actual table entries is not used to make it
>>> impossible to [wrongly, once the 128-bit form gets added] perform
>>> pointer arithmetic / array accesses on derived types.)
>>>
>>> Also move away from updating the entries piecemeal: Construct a full new
>>> entry, and write it out.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> I'm still not entirely convinced by extra union and containerof(), but
>> the result looks correct.
> And I'm still open to going the other way, if you're convinced that
> in update_intremap_entry() this
>
>      struct irte_basic basic = {
>          .flds = {
>              .remap_en = true,
>              .int_type = int_type,
>              .dm = dest_mode,
>              .dest = dest,
>              .vector = vector,
>          }
>      };
>
> (and similarly then for the 128-bit form, and of course .flds
> inserted at other use sites) is overall better than the current
> variant.

I've just experimented with the attached delta and it does compile in
CentOS 6, which is the usual culprit for problems in this area.

I do think the result is easier-to-read code, which I am definitely in
favour of.

My ack still stands in all affected patches, but ideally with this kind
of change folded in appropriately.

~Andrew

Attachment: reduced.patch
Description: Text Data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel