Re: [Xen-devel] [PATCH 2/9] AMD/IOMMU: use bit field for control register

["Jan Beulich" <JBeulich@suse.com>]

>>> On 18.06.19 at 11:54, <andrew.cooper3@citrix.com> wrote:
> On 13/06/2019 14:22, Jan Beulich wrote:
>> Also introduce a field in struct amd_iommu caching the most recently
>> written control register. All writes should now happen exclusively from
>> that cached value, such that it is guaranteed to be up to date.
>>
>> Take the opportunity and add further fields. Also convert a few boolean
>> function parameters to bool, such that use of !! can be avoided.
> 
> Critically also, some previous writel()'s have turned into writeq(),
> which needs calling out.

Sure, done.

>> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
>> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
>> @@ -295,38 +295,55 @@ struct amd_iommu_dte {
>>
>> +union amd_iommu_control {
>> +    uint64_t raw;
>> +    struct {
>> +        unsigned int iommu_en:1;
>> +        unsigned int ht_tun_en:1;
>> +        unsigned int event_log_en:1;
>> +        unsigned int event_int_en:1;
>> +        unsigned int com_wait_int_en:1;
>> +        unsigned int inv_timeout:3;
>> +        unsigned int pass_pw:1;
>> +        unsigned int res_pass_pw:1;
>> +        unsigned int coherent:1;
>> +        unsigned int isoc:1;
>> +        unsigned int cmd_buf_en:1;
>> +        unsigned int ppr_log_en:1;
>> +        unsigned int ppr_int_en:1;
>> +        unsigned int ppr_en:1;
>> +        unsigned int gt_en:1;
>> +        unsigned int ga_en:1;
>> +        unsigned int crw:4;
> 
> This field does have an assigned name, but is also documented as Res0
> for forwards compatibility.  I think this field wants handling
> consistently with...
> 
>> +        unsigned int smif_en:1;
>> +        unsigned int slf_wb_dis:1;
>> +        unsigned int smif_log_en:1;
>> +        unsigned int gam_en:3;
>> +        unsigned int ga_log_en:1;
>> +        unsigned int ga_int_en:1;
>> +        unsigned int dual_ppr_log_en:2;
>> +        unsigned int dual_event_log_en:2;
>> +        unsigned int dev_tbl_seg_en:3;
>> +        unsigned int priv_abrt_en:2;
>> +        unsigned int ppr_auto_rsp_en:1;
>> +        unsigned int marc_en:1;
>> +        unsigned int blk_stop_mrk_en:1;
>> +        unsigned int ppr_auto_rsp_aon:1;
>> +        unsigned int :2;
> 
> ... this, where you have dropped the DomainIDPNE bit (whatever the PN
> stands for).

I guess the work was done with an earlier version of the doc. I've
now added the missing name of the field.

Jan



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