Re: [RESEND PATCH 1/5] vtd: Ensure root entry is updated consistently
Teddy Astie <[email protected]> Wed, 5 Aug 2026 15:03:31 +0200
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <1785935013.8631fc262581453bbf619ec5b2062170.19fd205a597000e099@vates.tech> |
Le 04/08/2026 à 18:00, Jan Beulich a écrit :
> On 29.07.2026 12:05, Teddy Astie wrote:
>> --- a/xen/drivers/passthrough/vtd/iommu.c
>> +++ b/xen/drivers/passthrough/vtd/iommu.c
>> @@ -281,13 +281,13 @@ void free_pgtable_maddr(u64 maddr)
>> /* context entry handling */
>> static u64 bus_to_context_maddr(struct vtd_iommu *iommu, u8 bus)
>> {
>> - struct root_entry *root, *root_entries;
>> + struct root_entry root, *root_entries;
>> u64 maddr;
>>
>> ASSERT(spin_is_locked(&iommu->lock));
>> root_entries = (struct root_entry *)map_vtd_domain_page(iommu->root_maddr);
>> - root = &root_entries[bus];
>> - if ( !root_present(*root) )
>> + root.val = ACCESS_ONCE(root_entries[bus].val);
>
> I'm pretty concerned about this: You're reading only half of the entry here,
> and you're writing only half of it further down. The other half is reserved
> right now, but there's not even a comment being added to this effect. (Yet
> even with a comment, I'd still be concerned, just not as much.)
>
The idea is to match the original logic while making it always compile
correctly. As we don't interact with the other part of the root entry
(reserved, or upper context table with Scalable-Mode).
Ideally, it should be written using a bitfield structure instead, but
it's a much larger change.
>> @@ -295,11 +295,12 @@ static u64 bus_to_context_maddr(struct vtd_iommu *iommu, u8 bus)
>> unmap_vtd_domain_page(root_entries);
>> return 0;
>> }
>> - set_root_value(*root, maddr);
>> - set_root_present(*root);
>> - iommu_sync_cache(root, sizeof(struct root_entry));
>> + set_root_value(root, maddr);
>> + set_root_present(root);
>> + ACCESS_ONCE(root_entries[bus].val) = root.val;
>> + iommu_sync_cache(&root_entries[bus], sizeof(struct root_entry));
>
> sizeof(<expression>) please in favor of sizeof(<type>), whenever possible.
>
>> }
>> - maddr = (u64) get_context_addr(*root);
>> + maddr = (u64) get_context_addr(root);
>
> While there, drop the pointless casts, thus getting rid of two style issues
> as well?
>
Fixed locally.
> Jan
>
Teddy
OpenPGP_0x660FA9D102CBCFD0.asc
(application/pgp-keys, 2.4 KB)
-----BEGIN PGP PUBLIC KEY BLOCK----- xsDNBGn5sK8BDACuzSrrTjpVf4ay06OYB6yY0J1PqKffihoNMtrQRZjAHxoAPC7L TBVHV/XOZw5HJc+9R71z1JV+iYg6z3jPziGKzX8Fj3ZXlzJPmpf1PuETH3KdbvtJ T4ny+OGntnJntUoRKRPhTirr6yNeBk/637O3CQXjtqFUPZnko8OI/o1yawIBhJJA WicutjkkUgd28Bh6HV9EIumHtCBgn5/1A/fpm9624MMgYLsA8qjC4XsoovQvFCaO 8HEhvfzrrTZHjn/nPeB9SigxIxXW8YaTVqMdqul07o72m3eA2mf+LMu9a04FX/d4 wbxBLtELm+1jIrbtyaFZEMOLv/haSiS/Lj3btJH/EoucejoZ5SH49ksmVAmKOLkt OaTQ8b2gEvP7iaKiIiszCCtOSRohr+2GvDsDeLvVZnlR3I+SPhHar7TPKjFz0G3D PNolyjXywNqOAMpomSPi8lSwjAFsxOtQbcck/qRGRSNk4DAmH70pA+89MXfQXZ3q t1Q01B1+sU0I8xsAEQEAAc0kVGVkZHkgQXN0aWUgPHRlZGR5LmFzdGllQHZhdGVz LnRlY2g+wsENBBMBCAA3FiEEGAIew9LzHY3pdrqtZg+p0QLLz9AFAmn5sK8FCQWj moACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRBmD6nRAsvP0ID6DACGOktArFbLKHNz uyOVCskwfUZPla6Zpd3GZ8r61SrAKePIr2BnpgPkd0hV3bSRkRLIrgjzR2NRCzfp 0x0HfuhcYfAYPR46XHTvjaJEv99sT/vGUG1BZguYDOScSEpgSNaNlYum3RKZbMuR OxdK8G+YHccJY8PvWSq2K2yiae2KGiAv1yjnZxug9/PtDfX8vQFUSg2w1ukRDf50 wvDohN1zUQfFtofOP2xCRsDZiHAlQ0pF+aUjXQhPeP3IdpfWc8cyRLXF06Rk46YM YCytweGtGdHcqAfrVthl84129ZPN422k/voW0sm14gjYlGcTUwgnYlFRk2FLq0Qe KEDcS0aj3o3EVAQCrayoGzi1pnlIKE3PRGUcUzjGVvzQ/po24gOjwba9Egr/Wmu3 MQlx/7A8zT5QBzF/n+RYdLNQ0Eu6YnUwf0Z1uieqNaon+olyIRFiLb/hCZHO6ekN f5vrm2clHUbQAYaPQebknujoKBo6ZLHg0WM1gZS01Gz+aUpKsUfOwM0EafmwsAEM AKiQiZa3yQMmc/h3sDbfVHPSiBA4IMI/NAB7IotzPHq1GzCpsoVILAhF/INbWjxJ 3DbVf+en3/FvdVZg2S38xtnth0njNdlVKpyxm054phKjbdoFDwaknWolS4hrddTm etSG5/52AjtmPFtlXAk0NmLvfJnW3seXVQbgM7sW/MNXPP5UKDpkGnLhnvej+GU0 s3109sJeXT5ImVdphFs9cvyZyBT9t1PbRowv58EgV0zE4hbAeVkULAbxFV5b/ExT jjGVHoX7CVhWxvCiTqCUoXZRkUE9C3FnkzEFRkKbYu6NCfiHfEyB3Xyg9hfdrRgj MRq907zCof+nDtWxGz1MSEuvTj1g9GZ049Bennqzjc/Q+0ovXoK4jm+Py0FiUGUa A6yhexficjH+kCR/xDbVnWrMhSLB4AuTBT9HjfZI6gk3uYLhoT8Pig4/eVtR2Q1w ZIJsFToR6ofGuyECwFcs+PUXN7fmGRSiPXgjAr/zIUBdW0VWCE3OGPNqtRk2E5s6 IQARAQABwsD8BBgBCAAmFiEEGAIew9LzHY3pdrqtZg+p0QLLz9AFAmn5sLAFCQWj moACGwwACgkQZg+p0QLLz9DncQwAg76IehTemLIfrB8T9WIBZrI4kUV7G7a4rjiV oUiHYN5QwhnbZnsaJDlt+Ezoqy/510eo2bCSzvW5xXYPgyjcuOPwgQo1Qp764Qxy X6rld2f2RcWkDuBHun55ZWXjby8o21ginPRwruBVYY5rVf3DV1iBu4NurUeHtyFk /dS0XTOQi2wVUb17sW/+ybCEokdVacZGzOqP/OmwHrF8ylXlXnhQq6e3r+J+T8fu oGJelm/CJiMwyP6cEWE8sxVqX/iqwjwUYkuOCpE+lOWSvdNHgoEkWR0RXBPQjnGm LKbfTl/QDXLk6NP2/r9uxm2HL6Ei3QJKSEdrp+XZaVnk/OffO485NOTKwGOxyWb0 06cTMh53xPkAJFQu4Tvdj+odsHz88jqw5wfPG0BYWx0I/FspYj7N9kZR8ULR9nX0 LvpzJ/kB4NgHIUt8YtIL6ZSfM2dbF7fKzvx1UqFfvozJZwFzfEieJLXa4nlGgR6D x9fhaZEsniw8/bYgC3igkk5YJiOa =lUIA -----END PGP PUBLIC KEY BLOCK-----
OpenPGP_signature.asc
(application/pgp-signature, 665 B)
-----BEGIN PGP SIGNATURE----- wsD5BAABCAAjFiEEGAIew9LzHY3pdrqtZg+p0QLLz9AFAmpzNKQFAwAAAAAACgkQZg+p0QLLz9Cg ZAv8CejTcvdc2zq4YcBR1xmNNBKPx4ZBBe7qz5WPnbS+cOuKUWm5yxrbxwtWmItg9/L14Uho73Ws tKxdYHYanu0UzZ8gcGwtQ4nkhA+hKnDGlAJNp51msQCtYcfwWjw99y2crXOj4D5yE+v7nBSHHiM3 6HB6+Iv2k150B9Xwcerzj6AkWpYqzXDr3BEt7LDs1k+1jxvdsd87cuPDEMIgfkcSyQz1mpkXEagK g29wlRJ1CuXKtVzRGz6UydHevv3rQhDpCSS9/yf9us/B4JB1ondSTYOYEqjoJcOjy0lABKe61FT7 GM2D4TzkFqLe3JIUkdZ40/5GCsaBhN67EgCLhzu9Rewh0Y1U7WhInzZb2N4rdWrxLYvKRoQinu3u fRT5FbkZbuTK2CpTGfynmnf5IwSeUEVSeCaXak3yVT2JftFQEdvAQ1sTyskHBQNak8P70Jizi6zj JFME/W7nwPkf3m3z3v02CZJ+wWR/tEmTsYmBfEGHHPKyek5z6Absm8XjCacZ =emah -----END PGP SIGNATURE-----