* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Robin Murphy @ 2026-01-20 18:47 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Suzuki K Poulose, Aneesh Kumar K.V, linux-kernel, iommu,
linux-coco, Catalin Marinas, will, steven.price, Marek Szyprowski
In-Reply-To: <20260120175454.GA1325733@ziepe.ca>
On 2026-01-20 5:54 pm, Jason Gunthorpe wrote:
> On Tue, Jan 20, 2026 at 05:11:27PM +0000, Robin Murphy wrote:
>>> But you could make an argument that a trusted device won't DMA to
>>> shared memory, ie it would SWIOTLB to private memory if that is
>>> required.
>>
>> I don't think we can assume that any arbitrary trusted device is *never*
>> going to want to access shared memory in the Realm IPA space,
>
> Well, I can say it isn't supported with the DMA API we have today, so
> that's not *never* but at least for the present moment assuming that
> only private addresses are used with DMA would be consistent with the
> overall kernel capability.
>
> Certainly I think we have use cases for mixing traffic, and someone
> here is looking at what it would take to extend things to actually
> make it possible to reach into arbitrary shared memory with the DMA
> API..
>
>> and while it might technically be possible for a private SWIOTLB
>> buffer to handle that, we currently only have infrastructure that
>> assumes the opposite (i.e. that SWIOTLB buffers are shared for
>> bouncing untrusted DMA to/from private memory).
>
> We also don't support T=1 devices with the current kernel either, and
> the required behavior is exactly what a normal non-CC kernel does
> today. Basically, SWIOTLB should not be allocating or using shared
> memory with a T=1 device at all, and I think that is a important thing
> to have in the code for security.
>
> Anyhow, I'm just saying either you keep the limit as we have now or if
> the limit is relaxed for T=1 then it would make sense to fixup SWIOTLB
> to do traditional bouncing to avoid high (shared) addresses.
Indeed those are essentially all the same points I was making too - even
a T=1 device must support the full IPA range today, because if any
generated DMA address led to trying to use SWIOTLB via the current code
that would go horrifically wrong in ways likely to leak private data, or
at best fault at S2 and/or corrupt Realm memory (and at worst, all 3).
>> Thus for now, saying we can only promise to support DMA if the
>> device can access the whole IPA space itself is accurate.
>
> Right, that is where things are right now, and I don't think we should
> move away from those code limitations unless there are mitigations
> like bouncing..
>
>>> Otherwise these two limitations will exclude huge numbers of real
>>> devices from working with ARM CCA at all.
>>
>> Pretty sure the dependency on TDISP wins in that regard ;)
>
> You can use existing T=0 devices without TDISP
>
> And bolting a TDISP capable PCI IP onto a device with an addressing
> limit probably isn't going to fix the addressing limit. :(
>
>> However, assuming that Realms and RMMs might eventually come up with their
>> own attestation mechanisms for on-chip non-PCIe devices (and such devices
>> continue to have crippled DMA capabilities)
>
> The fabric isn't the only issue here, and even "PCIe" appearing
> devices don't necessarily run over real-PCIe and may have limited
> fabrics.
>
> There are enough important devices out there that have internal
> limitations, like registers and data structures that just cannot store
> the full 64 bit address space. HW folks have a big $$ incentive
> to take shortcuts like this..
Fair enough, guess I shall temper my optimism...
>> then the fact is still that DA requires an SMMU for S2, so at worst
>> there should always be the possibility for an RMM to offer S1 SMMU
>> support to the Realm, we're just not there yet.
>
> Having a S1 would help a T=1 device, but it doesn't do anything for
> the T=0 devices.
If we have an SMMU we have an SMMU - S1 for T=0 devices is just regular
VFIO/IOMMUFD in Non-Secure VA/IPA space, for which the VMM doesn't need
the RMM's help. I've long been taking it for granted that that one's a
non-issue ;)
The only thing we can't easily handle (and would rather avoid) is S1
translation for T=0 traffic from T=1 devices, since that would require
the Realm OS to comprehend the notion of a single device attached to two
different vSMMUs at once. Rather, to be workable I think we'd need to
keep the T=0 and T=1 states described as distinct devices - which
*could* then each be associated with "shared" (VMM-provided) and
"private" (RMM-provided) vSMMU instances respectively - and leave it as
the Realm driver's problem if it wants to coordinate enabling and using
both at the same time, kinda like the link aggregation model.
Thanks,
Robin.
> The other answer is to expect the VMM to limit the IPA size so that
> the IO devices can reach the full address space.
>
> Jason
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Jason Gunthorpe @ 2026-01-20 17:54 UTC (permalink / raw)
To: Robin Murphy
Cc: Suzuki K Poulose, Aneesh Kumar K.V, linux-kernel, iommu,
linux-coco, Catalin Marinas, will, steven.price, Marek Szyprowski
In-Reply-To: <a89e8fcc-29f0-4a6a-85f8-ca8ca41c53ce@arm.com>
On Tue, Jan 20, 2026 at 05:11:27PM +0000, Robin Murphy wrote:
> > But you could make an argument that a trusted device won't DMA to
> > shared memory, ie it would SWIOTLB to private memory if that is
> > required.
>
> I don't think we can assume that any arbitrary trusted device is *never*
> going to want to access shared memory in the Realm IPA space,
Well, I can say it isn't supported with the DMA API we have today, so
that's not *never* but at least for the present moment assuming that
only private addresses are used with DMA would be consistent with the
overall kernel capability.
Certainly I think we have use cases for mixing traffic, and someone
here is looking at what it would take to extend things to actually
make it possible to reach into arbitrary shared memory with the DMA
API..
> and while it might technically be possible for a private SWIOTLB
> buffer to handle that, we currently only have infrastructure that
> assumes the opposite (i.e. that SWIOTLB buffers are shared for
> bouncing untrusted DMA to/from private memory).
We also don't support T=1 devices with the current kernel either, and
the required behavior is exactly what a normal non-CC kernel does
today. Basically, SWIOTLB should not be allocating or using shared
memory with a T=1 device at all, and I think that is a important thing
to have in the code for security.
Anyhow, I'm just saying either you keep the limit as we have now or if
the limit is relaxed for T=1 then it would make sense to fixup SWIOTLB
to do traditional bouncing to avoid high (shared) addresses.
> Thus for now, saying we can only promise to support DMA if the
> device can access the whole IPA space itself is accurate.
Right, that is where things are right now, and I don't think we should
move away from those code limitations unless there are mitigations
like bouncing..
> > Otherwise these two limitations will exclude huge numbers of real
> > devices from working with ARM CCA at all.
>
> Pretty sure the dependency on TDISP wins in that regard ;)
You can use existing T=0 devices without TDISP
And bolting a TDISP capable PCI IP onto a device with an addressing
limit probably isn't going to fix the addressing limit. :(
> However, assuming that Realms and RMMs might eventually come up with their
> own attestation mechanisms for on-chip non-PCIe devices (and such devices
> continue to have crippled DMA capabilities)
The fabric isn't the only issue here, and even "PCIe" appearing
devices don't necessarily run over real-PCIe and may have limited
fabrics.
There are enough important devices out there that have internal
limitations, like registers and data structures that just cannot store
the full 64 bit address space. HW folks have a big $$ incentive
to take shortcuts like this..
> then the fact is still that DA requires an SMMU for S2, so at worst
> there should always be the possibility for an RMM to offer S1 SMMU
> support to the Realm, we're just not there yet.
Having a S1 would help a T=1 device, but it doesn't do anything for
the T=0 devices.
The other answer is to expect the VMM to limit the IPA size so that
the IO devices can reach the full address space.
Jason
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Robin Murphy @ 2026-01-20 17:11 UTC (permalink / raw)
To: Jason Gunthorpe, Suzuki K Poulose
Cc: Aneesh Kumar K.V, linux-kernel, iommu, linux-coco,
Catalin Marinas, will, steven.price, Marek Szyprowski
In-Reply-To: <20260120151127.GP961572@ziepe.ca>
On 2026-01-20 3:11 pm, Jason Gunthorpe wrote:
> On Tue, Jan 20, 2026 at 02:39:14PM +0000, Suzuki K Poulose wrote:
>>>>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>>>>> index 8e04f72baaa3..a5639e9415f5 100644
>>>>> --- a/kernel/dma/direct.c
>>>>> +++ b/kernel/dma/direct.c
>>>>> @@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
>>>>> /*
>>>>> * This check needs to be against the actual bit mask value, so use
>>>>> - * phys_to_dma_unencrypted() here so that the SME encryption mask isn't
>>>>> + * __phys_to_dma() here so that the arch specific encryption mask isn't
>>>>> * part of the check.
>>>>> */
>>>>> if (IS_ENABLED(CONFIG_ZONE_DMA))
>>>>> min_mask = min_t(u64, min_mask, zone_dma_limit);
>>>>> - return mask >= phys_to_dma_unencrypted(dev, min_mask);
>>>>> + return mask >= __phys_to_dma(dev, min_mask);
>>>>
>>>> This is wrong, isn't it ? For e.g., for CCA, even though the "Flag" is
>>>> added to the PA, it is really part of the actual "PA" and thus must be
>>>> checked against the full PA ?
>>>>
>>>
>>> That is true only when the device is operating in untrusted mode?. For a
>>> trusted device that mask is valid mask right?
>>
>> Irrespective of the mode in which the device is operating, the DMA
>> address must include the fully qualified "{I}PA" address, right ?
>> i.e., "the Unencrypted" bit is only a software construct and the full
>> PA must be used, irrespective of the mode of the device.
>
> But you could make an argument that a trusted device won't DMA to
> shared memory, ie it would SWIOTLB to private memory if that is
> required.
I don't think we can assume that any arbitrary trusted device is *never*
going to want to access shared memory in the Realm IPA space, and while
it might technically be possible for a private SWIOTLB buffer to handle
that, we currently only have infrastructure that assumes the opposite
(i.e. that SWIOTLB buffers are shared for bouncing untrusted DMA to/from
private memory). Thus for now, saying we can only promise to support DMA
if the device can access the whole IPA space itself is accurate.
> Otherwise these two limitations will exclude huge numbers of real
> devices from working with ARM CCA at all.
Pretty sure the dependency on TDISP wins in that regard ;)
However, assuming that Realms and RMMs might eventually come up with
their own attestation mechanisms for on-chip non-PCIe devices (and such
devices continue to have crippled DMA capabilities) then the fact is
still that DA requires an SMMU for S2, so at worst there should always
be the possibility for an RMM to offer S1 SMMU support to the Realm,
we're just not there yet.
Thanks,
Robin.
^ permalink raw reply
* SVSM Development Call January 21, 2026
From: Jörg Rödel @ 2026-01-20 17:09 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
Here is the call for agenda items for this weeks SVSM development call. Please
send any agenda items you have in mind as a reply to this email or raise them
in the meeting.
We will use the LF Zoom instance. Details of the meeting can be found in our
governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Jörg
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Jason Gunthorpe @ 2026-01-20 15:43 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: linux-kernel, iommu, linux-coco, Catalin Marinas, will,
robin.murphy, suzuki.poulose, steven.price, Marek Szyprowski
In-Reply-To: <yq5ams28jog8.fsf@kernel.org>
On Tue, Jan 20, 2026 at 08:55:43PM +0530, Aneesh Kumar K.V wrote:
> > The check in dma_direct_supported() is checking if the device's HW
> > capability can contain the range of dma_addr_t's the DMA API will
> > generate. Since it above is generating dma_addr_t's with the
> > PROT_NS_SHARED set, it is correct to check it against the mask.
> >
> > If the IOVA does not contain PROT_NS_SHARED then I would expect all of
> > the above to be removed too?
> >
> > Can you please explain what the probem is better?
>
> There is no specific problem identified. The motivation for this change
> is to ensure that the trusted device mask check is accurate.
Well, don't break untrusted in the process, and please explain
motivation in commit messages in future. Why things are being changed
is very important information.
> > I just had a long talk with our internal people about this very
> > subject and they were adament that the ARM design had the T=0 SMMU S2
> > configured so that the IOVA starts at PROT_NS_SHARED, not 0. I am
> > against this, I think it is a bad choice, and this patch is showing
> > exactly why :)
> >
> > IMHO you should map the T=0 S2 so that the IOVA starts at 0 and we
> > don't add PROT_NS_SHARED to the IOVA anyhwere.
>
> But how will we support a trusted device access to both shared and
> private memory? Commit 7d953a06241624ee2efb172d037a4168978f4147 goes
> into some details w.r.t why PROT_NS_SHARED was added to dma_addr_t.
The IO translation path used by trusted and untrusted devices are
completely different. You don't need a T=0 device to have the same S2
as a T=1 device. The commit doesn't explain anything, it is just
documenting the poor S2 conventions that CCA already adopted.
What should be done is that the T=0 untrusted translation path, which
can *never* access private memory anyhow, simply puts what the CPU
sees as the PROT_NS_SHARED region starting at IOVA 0. This path is
setup by the host VMM (ie qemu) and HW can support whatever
translation you want. There is no use case to ever not set
PROT_NS_SHARED for T=1 DMA so why break so many real devices by
requiring this useless bit?
The T=1 trusted translation path should have the RMM setup the S2 so
it mirrors the CPU S2 with the private memory at 0 and the shared
starting at PROT_NS_SHARED. An address width limited device would then
be unable to access shared memory without bouncing through private
memory with SWIOTLB, but it can probably access all the private
memory. The existing SWIOTLB address limit bouncing machinery is
designed to mitigate exactly this condition so it can be used to
bounce buffer from private memory to shared if that use case ever
comes up.
Given this little mistake has already happened I'd suggest that the
VMM/qemu be changed to double map the same T=0 space to both 0 and
PROT_NS_SHARED, and have a discovery bit in the guest that it is
allowed to use 0 based IOVA for T=0 DMA.
Then you can fixuup the DMA API calls so that:
1) If an old VMM continue to use PROT_NS_SHARED for T=0 and continue
to check it against the DMA limit, de-supporting address width
limited HW
2) If a new VMM drop PROT_NS_SHARED for T=0 and check it. Since
everything must use SWIOTLB ensure that SWIOTLB allocated addresses
within the device support range, as it already is able to do.
3) For a T=1 device assume it will not DMA to shared memory, we don't
have any way to describe that in the DMA API right now. Check the
native physical address against the limit and SWIOTLB bounce to
private memory for outside the limit.
This way CCA would work on alot more actual HW :\
Jason
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Aneesh Kumar K.V @ 2026-01-20 15:25 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-kernel, iommu, linux-coco, Catalin Marinas, will,
robin.murphy, suzuki.poulose, steven.price, Marek Szyprowski
In-Reply-To: <20260120132657.GO961572@ziepe.ca>
Jason Gunthorpe <jgg@ziepe.ca> writes:
> On Tue, Jan 20, 2026 at 12:12:54PM +0530, Aneesh Kumar K.V (Arm) wrote:
>> On systems that apply an address encryption tag or mask to DMA addresses,
>> DMA mask validation must be performed against the canonical DMA address.
>> Using a non-canonical (e.g. encrypted or unencrypted) DMA address
>> can incorrectly fail capability checks, since architecture-specific
>> encryption bits are not part of the device’s actual DMA addressing
>> capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
>> addresses.
>
> Huh?
>
> static inline dma_addr_t phys_to_dma_direct(struct device *dev,
> phys_addr_t phys)
> {
> if (force_dma_unencrypted(dev))
> return phys_to_dma_unencrypted(dev, phys);
> return phys_to_dma(dev, phys);
> }
>
> dma_addr_t swiotlb_map(struct device *dev, phys_addr_t paddr, size_t size,
> enum dma_data_direction dir, unsigned long attrs)
> {
> [..]
>
> /* Ensure that the address returned is DMA'ble */
> dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
> [..]
> return dma_addr;
> }
>
> The check in dma_direct_supported() is checking if the device's HW
> capability can contain the range of dma_addr_t's the DMA API will
> generate. Since it above is generating dma_addr_t's with the
> PROT_NS_SHARED set, it is correct to check it against the mask.
>
> If the IOVA does not contain PROT_NS_SHARED then I would expect all of
> the above to be removed too?
>
> Can you please explain what the probem is better?
>
There is no specific problem identified. The motivation for this change
is to ensure that the trusted device mask check is accurate.
>
> I just had a long talk with our internal people about this very
> subject and they were adament that the ARM design had the T=0 SMMU S2
> configured so that the IOVA starts at PROT_NS_SHARED, not 0. I am
> against this, I think it is a bad choice, and this patch is showing
> exactly why :)
>
> IMHO you should map the T=0 S2 so that the IOVA starts at 0 and we
> don't add PROT_NS_SHARED to the IOVA anyhwere.
>
But how will we support a trusted device access to both shared and
private memory? Commit 7d953a06241624ee2efb172d037a4168978f4147 goes
into some details w.r.t why PROT_NS_SHARED was added to dma_addr_t.
>
> This logic is going to be wrong for T=1 DMA to private memory though.
>
> Jason
-aneesh
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Jason Gunthorpe @ 2026-01-20 15:11 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: Aneesh Kumar K.V, linux-kernel, iommu, linux-coco,
Catalin Marinas, will, robin.murphy, steven.price,
Marek Szyprowski
In-Reply-To: <0da8b73c-5bec-44c3-9902-221a11142c34@arm.com>
On Tue, Jan 20, 2026 at 02:39:14PM +0000, Suzuki K Poulose wrote:
> > > > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> > > > index 8e04f72baaa3..a5639e9415f5 100644
> > > > --- a/kernel/dma/direct.c
> > > > +++ b/kernel/dma/direct.c
> > > > @@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
> > > > /*
> > > > * This check needs to be against the actual bit mask value, so use
> > > > - * phys_to_dma_unencrypted() here so that the SME encryption mask isn't
> > > > + * __phys_to_dma() here so that the arch specific encryption mask isn't
> > > > * part of the check.
> > > > */
> > > > if (IS_ENABLED(CONFIG_ZONE_DMA))
> > > > min_mask = min_t(u64, min_mask, zone_dma_limit);
> > > > - return mask >= phys_to_dma_unencrypted(dev, min_mask);
> > > > + return mask >= __phys_to_dma(dev, min_mask);
> > >
> > > This is wrong, isn't it ? For e.g., for CCA, even though the "Flag" is
> > > added to the PA, it is really part of the actual "PA" and thus must be
> > > checked against the full PA ?
> > >
> >
> > That is true only when the device is operating in untrusted mode?. For a
> > trusted device that mask is valid mask right?
>
> Irrespective of the mode in which the device is operating, the DMA
> address must include the fully qualified "{I}PA" address, right ?
> i.e., "the Unencrypted" bit is only a software construct and the full
> PA must be used, irrespective of the mode of the device.
But you could make an argument that a trusted device won't DMA to
shared memory, ie it would SWIOTLB to private memory if that is
required.
Otherwise these two limitations will exclude huge numbers of real
devices from working with ARM CCA at all.
Jason
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Suzuki K Poulose @ 2026-01-20 14:39 UTC (permalink / raw)
To: Aneesh Kumar K.V, linux-kernel, iommu, linux-coco
Cc: Catalin Marinas, will, robin.murphy, jgg, steven.price,
Marek Szyprowski
In-Reply-To: <yq5atswgjrkp.fsf@kernel.org>
On 20/01/2026 14:18, Aneesh Kumar K.V wrote:
> Suzuki K Poulose <suzuki.poulose@arm.com> writes:
>
>> On 20/01/2026 06:42, Aneesh Kumar K.V (Arm) wrote:
>>> On systems that apply an address encryption tag or mask to DMA addresses,
>>> DMA mask validation must be performed against the canonical DMA address.
>>> Using a non-canonical (e.g. encrypted or unencrypted) DMA address
>>> can incorrectly fail capability checks, since architecture-specific
>>> encryption bits are not part of the device’s actual DMA addressing
>>> capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
>>> addresses.
>>>
>>> Fix this by validating device DMA masks against __phys_to_dma(), ensuring
>>> that the architecture encryption mask does not influence the check.
>>>
>>> Fixes: b66e2ee7b6c8 ("dma: Introduce generic dma_addr_*crypted helpers")
>>
>>
>>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>>> ---
>>> kernel/dma/direct.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>>> index 8e04f72baaa3..a5639e9415f5 100644
>>> --- a/kernel/dma/direct.c
>>> +++ b/kernel/dma/direct.c
>>> @@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
>>>
>>> /*
>>> * This check needs to be against the actual bit mask value, so use
>>> - * phys_to_dma_unencrypted() here so that the SME encryption mask isn't
>>> + * __phys_to_dma() here so that the arch specific encryption mask isn't
>>> * part of the check.
>>> */
>>> if (IS_ENABLED(CONFIG_ZONE_DMA))
>>> min_mask = min_t(u64, min_mask, zone_dma_limit);
>>> - return mask >= phys_to_dma_unencrypted(dev, min_mask);
>>> + return mask >= __phys_to_dma(dev, min_mask);
>>
>> This is wrong, isn't it ? For e.g., for CCA, even though the "Flag" is
>> added to the PA, it is really part of the actual "PA" and thus must be
>> checked against the full PA ?
>>
>
> That is true only when the device is operating in untrusted mode?. For a
> trusted device that mask is valid mask right?
Irrespective of the mode in which the device is operating, the DMA
address must include the fully qualified "{I}PA" address, right ?
i.e., "the Unencrypted" bit is only a software construct and the full
PA must be used, irrespective of the mode of the device.
Suzuki
>
> -aneesh
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Aneesh Kumar K.V @ 2026-01-20 14:25 UTC (permalink / raw)
To: Robin Murphy, Suzuki K Poulose, linux-kernel, iommu, linux-coco
Cc: Catalin Marinas, will, jgg, steven.price, Marek Szyprowski
In-Reply-To: <893a6503-d33f-41d8-8966-19b9f251d9da@arm.com>
Robin Murphy <robin.murphy@arm.com> writes:
> On 2026-01-20 9:59 am, Suzuki K Poulose wrote:
>> On 20/01/2026 06:42, Aneesh Kumar K.V (Arm) wrote:
>>> On systems that apply an address encryption tag or mask to DMA addresses,
>>> DMA mask validation must be performed against the canonical DMA address.
>>> Using a non-canonical (e.g. encrypted or unencrypted) DMA address
>>> can incorrectly fail capability checks, since architecture-specific
>>> encryption bits are not part of the device’s actual DMA addressing
>>> capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
>>> addresses.
>>>
>>> Fix this by validating device DMA masks against __phys_to_dma(), ensuring
>>> that the architecture encryption mask does not influence the check.
>>>
>>> Fixes: b66e2ee7b6c8 ("dma: Introduce generic dma_addr_*crypted helpers")
>>
>>
>>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>>> ---
>>> kernel/dma/direct.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>>> index 8e04f72baaa3..a5639e9415f5 100644
>>> --- a/kernel/dma/direct.c
>>> +++ b/kernel/dma/direct.c
>>> @@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64
>>> mask)
>>> /*
>>> * This check needs to be against the actual bit mask value, so use
>>> - * phys_to_dma_unencrypted() here so that the SME encryption mask
>>> isn't
>>> + * __phys_to_dma() here so that the arch specific encryption mask
>>> isn't
>>> * part of the check.
>>> */
>>> if (IS_ENABLED(CONFIG_ZONE_DMA))
>>> min_mask = min_t(u64, min_mask, zone_dma_limit);
>>> - return mask >= phys_to_dma_unencrypted(dev, min_mask);
>>> + return mask >= __phys_to_dma(dev, min_mask);
>>
>> This is wrong, isn't it ? For e.g., for CCA, even though the "Flag" is
>> added to the PA, it is really part of the actual "PA" and thus must be
>> checked against the full PA ?
>
> Yes, it's much the same as for AMD SEV (albeit the other way round) -
> the encryption/decryption bit is part of the DMA address because it
> needs to be driven by the device, so it is crucial that the device's DMA
> mask is capable of expressing that.
>
Commit c92a54cfa0257e8ffd66b2a17d49e9c0bd4b769f explains these details.
I was wondering whether the DMA-enable operation should live outside the
set_mask operation. Conceptually, set_mask should be derived purely from
max_pfn, while the DMA enablement path could additionally verify whether
the device requires access to an alias address, depending on whether it
is operating in trusted or untrusted mode?
>
> Hence, as I think we've discussed before, for CCA we can't really
> support 32-bit devices doing DMA to shared pages at all, unless the
> whole VM is limited to a 31-bit IPA space.
>
-aneesh
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Aneesh Kumar K.V @ 2026-01-20 14:18 UTC (permalink / raw)
To: Suzuki K Poulose, linux-kernel, iommu, linux-coco
Cc: Catalin Marinas, will, robin.murphy, jgg, steven.price,
Marek Szyprowski
In-Reply-To: <2a0b6d1b-875a-4075-8fc9-a8534afc9168@arm.com>
Suzuki K Poulose <suzuki.poulose@arm.com> writes:
> On 20/01/2026 06:42, Aneesh Kumar K.V (Arm) wrote:
>> On systems that apply an address encryption tag or mask to DMA addresses,
>> DMA mask validation must be performed against the canonical DMA address.
>> Using a non-canonical (e.g. encrypted or unencrypted) DMA address
>> can incorrectly fail capability checks, since architecture-specific
>> encryption bits are not part of the device’s actual DMA addressing
>> capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
>> addresses.
>>
>> Fix this by validating device DMA masks against __phys_to_dma(), ensuring
>> that the architecture encryption mask does not influence the check.
>>
>> Fixes: b66e2ee7b6c8 ("dma: Introduce generic dma_addr_*crypted helpers")
>
>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>> kernel/dma/direct.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index 8e04f72baaa3..a5639e9415f5 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
>>
>> /*
>> * This check needs to be against the actual bit mask value, so use
>> - * phys_to_dma_unencrypted() here so that the SME encryption mask isn't
>> + * __phys_to_dma() here so that the arch specific encryption mask isn't
>> * part of the check.
>> */
>> if (IS_ENABLED(CONFIG_ZONE_DMA))
>> min_mask = min_t(u64, min_mask, zone_dma_limit);
>> - return mask >= phys_to_dma_unencrypted(dev, min_mask);
>> + return mask >= __phys_to_dma(dev, min_mask);
>
> This is wrong, isn't it ? For e.g., for CCA, even though the "Flag" is
> added to the PA, it is really part of the actual "PA" and thus must be
> checked against the full PA ?
>
That is true only when the device is operating in untrusted mode?. For a
trusted device that mask is valid mask right?
-aneesh
^ permalink raw reply
* Re: [RFC PATCH 4/5] firmware: smccc: register as platform driver
From: Salman Nabi @ 2026-01-20 14:07 UTC (permalink / raw)
To: Vedashree Vidwans, sudeep.holla, andre.przywara, lpieralisi,
mark.rutland
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <20251208221319.1524888-5-vvidwans@nvidia.com>
Hi Veda,
On 12/8/25 22:13, Vedashree Vidwans wrote:
> - Update driver to be in-built kernel module. This will ensure driver is
> installed in kernel and would not require any user intervention.
> - Register the LFA driver as a platform driver corresponding to
> 'armhf000' device. The driver will be invoked when the device is
> detected on a platform.
> - Add functionality to register LFA interrupt in the driver probe().
> This LFA IRQ number will be retrived from the LFA device node.
> - On IRQ, driver will query FW component details and trigger activation
> of capable and pending FW component. The driver will loop to update FW
> component details after every successful FW component activation.
> - Mutex synchronization is implemented to avoid concurrent LFA updates
> through interrupt and sysfs interfaces.
>
> Device node snippet from LFA spec[1]:
> fwu0 {
> compatible = "arm,armhf000";
> memory-region = <&fwu_payload>;
> interrupt-parent = <&ic>;
> interrupts = <0 100 1>; // SPI, Interrupt #100, Edge Rising
> };
>
> [1] https://developer.arm.com/documentation/den0147/latest/
>
> Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
> ---
> drivers/firmware/smccc/Kconfig | 3 +-
> drivers/firmware/smccc/lfa_fw.c | 124 +++++++++++++++++++++++++++++++-
> 2 files changed, 125 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/smccc/Kconfig b/drivers/firmware/smccc/Kconfig
> index 48b98c14f770..c21be43fbfed 100644
> --- a/drivers/firmware/smccc/Kconfig
> +++ b/drivers/firmware/smccc/Kconfig
> @@ -25,8 +25,9 @@ config ARM_SMCCC_SOC_ID
> platforms providing some sysfs information about the SoC variant.
>
> config ARM_LFA
> - tristate "Arm Live Firmware activation support"
> + bool "Arm Live Firmware activation support"
I think the LFA driver is a strong candidate for a module rather than built-in only. If it ever needs to be built-in, we can always set its default state to "y". Kernel best practices prefer using tristate because it provides flexibility, unless the code cannot be modularized, for example, when it must run before the module loader is available.
Just an FYI, for built-in code the driver will need to be refactored, for example, the __exit call will need to be removed and the __init call will need to be changed, see here: https://lkml.org/lkml/2015/5/10/125
Many thanks,
Salman
> depends on HAVE_ARM_SMCCC_DISCOVERY
> + default y
> help
> Include support for triggering Live Firmware Activation, which
> allows to upgrade certain firmware components without a reboot.
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index 0e420cefa260..24916fc53420 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -19,7 +19,12 @@
> #include <linux/nmi.h>
> #include <linux/ktime.h>
> #include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/acpi.h>
> +#include <linux/interrupt.h>
> +#include <linux/mutex.h>
>
> +#define DRIVER_NAME "ARM_LFA"
> #define LFA_ERROR_STRING(name) \
> [name] = #name
> #undef pr_fmt
> @@ -129,6 +134,7 @@ static const struct fw_image_uuid {
> };
>
> static struct kobject *lfa_dir;
> +static DEFINE_MUTEX(lfa_lock);
>
> static int get_nr_lfa_components(void)
> {
> @@ -374,17 +380,23 @@ static ssize_t activate_store(struct kobject *kobj, struct kobj_attribute *attr,
> image_attrs[LFA_ATTR_ACTIVATE]);
> int ret;
>
> + if (!mutex_trylock(&lfa_lock)) {
> + pr_err("Mutex locked, try again");
> + return -EAGAIN;
> + }
> +
> ret = activate_fw_image(attrs);
> if (ret) {
> pr_err("Firmware activation failed: %s\n",
> lfa_error_strings[-ret]);
> -
> + mutex_unlock(&lfa_lock);
> return -ECANCELED;
> }
>
> pr_info("Firmware activation succeeded\n");
>
> /* TODO: refresh image flags here*/
> + mutex_unlock(&lfa_lock);
> return count;
> }
>
> @@ -510,6 +522,106 @@ static int create_fw_images_tree(void)
> return 0;
> }
>
> +static irqreturn_t lfa_irq_thread(int irq, void *data)
> +{
> + struct image_props *attrs = NULL;
> + int ret;
> + int num_of_components, curr_component;
> +
> + mutex_lock(&lfa_lock);
> +
> + /*
> + * As per LFA spec, after activation of a component, the caller
> + * is expected to re-enumerate the component states (using
> + * LFA_GET_INFO then LFA_GET_INVENTORY).
> + * Hence we need an unconditional loop.
> + */
> +
> + do {
> + /* TODO: refresh image flags here */
> + /* If refresh fails goto exit_unlock */
> +
> + /* Initialize counters to track list traversal */
> + num_of_components = get_nr_lfa_components();
> + curr_component = 0;
> +
> + /* Execute PRIME and ACTIVATE for activable FW component */
> + list_for_each_entry(attrs, &lfa_fw_images, image_node) {
> + curr_component++;
> + if ((!attrs->activation_capable) || (!attrs->activation_pending)) {
> + /* LFA not applicable for this FW component */
> + continue;
> + }
> +
> + ret = activate_fw_image(attrs);
> + if (ret) {
> + pr_err("Firmware %s activation failed: %s\n",
> + attrs->image_name, lfa_error_strings[-ret]);
> + goto exit_unlock;
> + }
> +
> + pr_info("Firmware %s activation succeeded", attrs->image_name);
> + /* Refresh FW component details */
> + break;
> + }
> + } while (curr_component < num_of_components);
> +
> + /* TODO: refresh image flags here */
> + /* If refresh fails goto exit_unlock */
> +
> +exit_unlock:
> + mutex_unlock(&lfa_lock);
> + return IRQ_HANDLED;
> +}
> +
> +static int __init lfa_probe(struct platform_device *pdev)
> +{
> + int err;
> + unsigned int irq;
> +
> + err = platform_get_irq_byname_optional(pdev, "fw-store-updated-interrupt");
> + if (err < 0)
> + err = platform_get_irq(pdev, 0);
> + if (err < 0) {
> + pr_err("Interrupt not found, functionality will be unavailable.");
> +
> + /* Bail out without failing the driver. */
> + return 0;
> + }
> + irq = err;
> +
> + err = request_threaded_irq(irq, NULL, lfa_irq_thread, IRQF_ONESHOT, DRIVER_NAME, NULL);
> + if (err != 0) {
> + pr_err("Interrupt setup failed, functionality will be unavailable.");
> +
> + /* Bail out without failing the driver. */
> + return 0;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id lfa_of_ids[] = {
> + { .compatible = "arm,armhf000", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, lfa_of_ids);
> +
> +static const struct acpi_device_id lfa_acpi_ids[] = {
> + {"ARMHF000"},
> + {},
> +};
> +MODULE_DEVICE_TABLE(acpi, lfa_acpi_ids);
> +
> +static struct platform_driver lfa_driver = {
> + .probe = lfa_probe,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = lfa_of_ids,
> + .acpi_match_table = ACPI_PTR(lfa_acpi_ids),
> + },
> +};
> +
> static int __init lfa_init(void)
> {
> struct arm_smccc_1_2_regs reg = { 0 };
> @@ -536,22 +648,32 @@ static int __init lfa_init(void)
> pr_info("Arm Live Firmware Activation (LFA): detected v%ld.%ld\n",
> reg.a0 >> 16, reg.a0 & 0xffff);
>
> + err = platform_driver_register(&lfa_driver);
> + if (err < 0)
> + pr_err("Platform driver register failed");
> +
> lfa_dir = kobject_create_and_add("lfa", firmware_kobj);
> if (!lfa_dir)
> return -ENOMEM;
>
> + mutex_lock(&lfa_lock);
> err = create_fw_images_tree();
> if (err != 0)
> kobject_put(lfa_dir);
>
> + mutex_unlock(&lfa_lock);
> return err;
> }
> module_init(lfa_init);
>
> static void __exit lfa_exit(void)
> {
> + mutex_lock(&lfa_lock);
> clean_fw_images_tree();
> + mutex_unlock(&lfa_lock);
> +
> kobject_put(lfa_dir);
> + platform_driver_unregister(&lfa_driver);
> }
> module_exit(lfa_exit);
>
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Jason Gunthorpe @ 2026-01-20 13:26 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm)
Cc: linux-kernel, iommu, linux-coco, Catalin Marinas, will,
robin.murphy, suzuki.poulose, steven.price, Marek Szyprowski
In-Reply-To: <20260120064255.179425-1-aneesh.kumar@kernel.org>
On Tue, Jan 20, 2026 at 12:12:54PM +0530, Aneesh Kumar K.V (Arm) wrote:
> On systems that apply an address encryption tag or mask to DMA addresses,
> DMA mask validation must be performed against the canonical DMA address.
> Using a non-canonical (e.g. encrypted or unencrypted) DMA address
> can incorrectly fail capability checks, since architecture-specific
> encryption bits are not part of the device’s actual DMA addressing
> capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
> addresses.
Huh?
static inline dma_addr_t phys_to_dma_direct(struct device *dev,
phys_addr_t phys)
{
if (force_dma_unencrypted(dev))
return phys_to_dma_unencrypted(dev, phys);
return phys_to_dma(dev, phys);
}
dma_addr_t swiotlb_map(struct device *dev, phys_addr_t paddr, size_t size,
enum dma_data_direction dir, unsigned long attrs)
{
[..]
/* Ensure that the address returned is DMA'ble */
dma_addr = phys_to_dma_unencrypted(dev, swiotlb_addr);
[..]
return dma_addr;
}
The check in dma_direct_supported() is checking if the device's HW
capability can contain the range of dma_addr_t's the DMA API will
generate. Since it above is generating dma_addr_t's with the
PROT_NS_SHARED set, it is correct to check it against the mask.
If the IOVA does not contain PROT_NS_SHARED then I would expect all of
the above to be removed too?
Can you please explain what the probem is better?
I just had a long talk with our internal people about this very
subject and they were adament that the ARM design had the T=0 SMMU S2
configured so that the IOVA starts at PROT_NS_SHARED, not 0. I am
against this, I think it is a bad choice, and this patch is showing
exactly why :)
IMHO you should map the T=0 S2 so that the IOVA starts at 0 and we
don't add PROT_NS_SHARED to the IOVA anyhwere.
This logic is going to be wrong for T=1 DMA to private memory though.
Jason
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Robin Murphy @ 2026-01-20 11:59 UTC (permalink / raw)
To: Suzuki K Poulose, Aneesh Kumar K.V (Arm), linux-kernel, iommu,
linux-coco
Cc: Catalin Marinas, will, jgg, steven.price, Marek Szyprowski
In-Reply-To: <2a0b6d1b-875a-4075-8fc9-a8534afc9168@arm.com>
On 2026-01-20 9:59 am, Suzuki K Poulose wrote:
> On 20/01/2026 06:42, Aneesh Kumar K.V (Arm) wrote:
>> On systems that apply an address encryption tag or mask to DMA addresses,
>> DMA mask validation must be performed against the canonical DMA address.
>> Using a non-canonical (e.g. encrypted or unencrypted) DMA address
>> can incorrectly fail capability checks, since architecture-specific
>> encryption bits are not part of the device’s actual DMA addressing
>> capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
>> addresses.
>>
>> Fix this by validating device DMA masks against __phys_to_dma(), ensuring
>> that the architecture encryption mask does not influence the check.
>>
>> Fixes: b66e2ee7b6c8 ("dma: Introduce generic dma_addr_*crypted helpers")
>
>
>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> ---
>> kernel/dma/direct.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
>> index 8e04f72baaa3..a5639e9415f5 100644
>> --- a/kernel/dma/direct.c
>> +++ b/kernel/dma/direct.c
>> @@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64
>> mask)
>> /*
>> * This check needs to be against the actual bit mask value, so use
>> - * phys_to_dma_unencrypted() here so that the SME encryption mask
>> isn't
>> + * __phys_to_dma() here so that the arch specific encryption mask
>> isn't
>> * part of the check.
>> */
>> if (IS_ENABLED(CONFIG_ZONE_DMA))
>> min_mask = min_t(u64, min_mask, zone_dma_limit);
>> - return mask >= phys_to_dma_unencrypted(dev, min_mask);
>> + return mask >= __phys_to_dma(dev, min_mask);
>
> This is wrong, isn't it ? For e.g., for CCA, even though the "Flag" is
> added to the PA, it is really part of the actual "PA" and thus must be
> checked against the full PA ?
Yes, it's much the same as for AMD SEV (albeit the other way round) -
the encryption/decryption bit is part of the DMA address because it
needs to be driven by the device, so it is crucial that the device's DMA
mask is capable of expressing that.
Hence, as I think we've discussed before, for CCA we can't really
support 32-bit devices doing DMA to shared pages at all, unless the
whole VM is limited to a 31-bit IPA space.
Thanks,
Robin.
>
> Suzuki
>
>
>> }
>> static const struct bus_dma_region *dma_find_range(struct device *dev,
>
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: kernel test robot @ 2026-01-20 10:59 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), linux-kernel, iommu, linux-coco
Cc: oe-kbuild-all, Catalin Marinas, will, robin.murphy,
suzuki.poulose, jgg, steven.price, Marek Szyprowski,
Aneesh Kumar K.V (Arm)
In-Reply-To: <20260120064255.179425-1-aneesh.kumar@kernel.org>
Hi Aneesh,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.19-rc6 next-20260119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Aneesh-Kumar-K-V-Arm/dma-direct-Make-phys_to_dma-pick-encrypted-vs-unencrypted-per-device/20260120-145025
base: linus/master
patch link: https://lore.kernel.org/r/20260120064255.179425-1-aneesh.kumar%40kernel.org
patch subject: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
config: powerpc-allnoconfig (https://download.01.org/0day-ci/archive/20260120/202601201822.0I4WAqVW-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601201822.0I4WAqVW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601201822.0I4WAqVW-lkp@intel.com/
All errors (new ones prefixed by >>):
kernel/dma/direct.c: In function 'dma_direct_supported':
>> kernel/dma/direct.c:564:24: error: implicit declaration of function '__phys_to_dma'; did you mean 'phys_to_dma'? [-Wimplicit-function-declaration]
564 | return mask >= __phys_to_dma(dev, min_mask);
| ^~~~~~~~~~~~~
| phys_to_dma
vim +564 kernel/dma/direct.c
543
544 int dma_direct_supported(struct device *dev, u64 mask)
545 {
546 u64 min_mask = (max_pfn - 1) << PAGE_SHIFT;
547
548 /*
549 * Because 32-bit DMA masks are so common we expect every architecture
550 * to be able to satisfy them - either by not supporting more physical
551 * memory, or by providing a ZONE_DMA32. If neither is the case, the
552 * architecture needs to use an IOMMU instead of the direct mapping.
553 */
554 if (mask >= DMA_BIT_MASK(32))
555 return 1;
556
557 /*
558 * This check needs to be against the actual bit mask value, so use
559 * __phys_to_dma() here so that the arch specific encryption mask isn't
560 * part of the check.
561 */
562 if (IS_ENABLED(CONFIG_ZONE_DMA))
563 min_mask = min_t(u64, min_mask, zone_dma_limit);
> 564 return mask >= __phys_to_dma(dev, min_mask);
565 }
566
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 2/2] dma-direct: Make phys_to_dma() pick encrypted vs unencrypted per device
From: kernel test robot @ 2026-01-20 10:49 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), linux-kernel, iommu, linux-coco
Cc: oe-kbuild-all, Catalin Marinas, will, robin.murphy,
suzuki.poulose, jgg, steven.price, Marek Szyprowski,
Aneesh Kumar K.V (Arm)
In-Reply-To: <20260120064255.179425-2-aneesh.kumar@kernel.org>
Hi Aneesh,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.19-rc6 next-20260119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Aneesh-Kumar-K-V-Arm/dma-direct-Make-phys_to_dma-pick-encrypted-vs-unencrypted-per-device/20260120-145025
base: linus/master
patch link: https://lore.kernel.org/r/20260120064255.179425-2-aneesh.kumar%40kernel.org
patch subject: [PATCH 2/2] dma-direct: Make phys_to_dma() pick encrypted vs unencrypted per device
config: alpha-allnoconfig (https://download.01.org/0day-ci/archive/20260120/202601201857.7LKBY5dB-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601201857.7LKBY5dB-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601201857.7LKBY5dB-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from kernel/dma/direct.h:10,
from kernel/dma/mapping.c:19:
include/linux/dma-direct.h: In function 'phys_to_dma':
>> include/linux/dma-direct.h:103:13: error: implicit declaration of function 'force_dma_unencrypted'; did you mean 'phys_to_dma_unencrypted'? [-Wimplicit-function-declaration]
103 | if (force_dma_unencrypted(dev))
| ^~~~~~~~~~~~~~~~~~~~~
| phys_to_dma_unencrypted
include/linux/dma-direct.h: At top level:
>> include/linux/dma-direct.h:125:20: error: conflicting types for 'force_dma_unencrypted'; have 'bool(struct device *)' {aka '_Bool(struct device *)'}
125 | static inline bool force_dma_unencrypted(struct device *dev)
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/dma-direct.h:103:13: note: previous implicit declaration of 'force_dma_unencrypted' with type 'int()'
103 | if (force_dma_unencrypted(dev))
| ^~~~~~~~~~~~~~~~~~~~~
vim +103 include/linux/dma-direct.h
102
> 103 if (force_dma_unencrypted(dev))
104 return phys_to_dma_unencrypted(dev, paddr);
105 return phys_to_dma_encrypted(dev, paddr);
106 }
107
108 static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
109 {
110 phys_addr_t paddr;
111
112 dma_addr = dma_addr_canonical(dma_addr);
113 if (dev->dma_range_map)
114 paddr = translate_dma_to_phys(dev, dma_addr);
115 else
116 paddr = dma_addr;
117
118 return paddr;
119 }
120 #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
121
122 #ifdef CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED
123 bool force_dma_unencrypted(struct device *dev);
124 #else
> 125 static inline bool force_dma_unencrypted(struct device *dev)
126 {
127 return false;
128 }
129 #endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
130
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 11/16] KVM: TDX: Add x86 ops for external spt cache
From: Huang, Kai @ 2026-01-20 10:00 UTC (permalink / raw)
To: Zhao, Yan Y
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, Wu, Binbin, kas@kernel.org, seanjc@google.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
pbonzini@redhat.com, Yamahata, Isaku, tglx@linutronix.de,
Annapurve, Vishal, Gao, Chao, Edgecombe, Rick P, bp@alien8.de,
x86@kernel.org
In-Reply-To: <aW9IetVmF3pIVFRl@yzhao56-desk.sh.intel.com>
On Tue, 2026-01-20 at 17:18 +0800, Yan Zhao wrote:
> > When sp->spt is allocated via per-vCPU mmu_shadow_page_cache, it is
> > actually initialized to SHADOW_NONPRESENT_VALUE:
> >
> > vcpu->arch.mmu_shadow_page_cache.init_value =
> > SHADOW_NONPRESENT_VALUE;
> >
> > So the way sp->spt is allocated in tdp_mmu_alloc_sp_for_split() is
> > actually broken IMHO because entries in sp->spt is never initialized.
> The sp->spt allocated in tdp_mmu_alloc_sp_for_split() is initialized in
> tdp_mmu_split_huge_page()...
Oh right, we already have a huge SPTE to copy from in this case, so no
problem here, but seems the inconsistency is still there to me.
^ permalink raw reply
* Re: [PATCH 1/2] dma-direct: Validate DMA mask against canonical DMA addresses
From: Suzuki K Poulose @ 2026-01-20 9:59 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), linux-kernel, iommu, linux-coco
Cc: Catalin Marinas, will, robin.murphy, jgg, steven.price,
Marek Szyprowski
In-Reply-To: <20260120064255.179425-1-aneesh.kumar@kernel.org>
On 20/01/2026 06:42, Aneesh Kumar K.V (Arm) wrote:
> On systems that apply an address encryption tag or mask to DMA addresses,
> DMA mask validation must be performed against the canonical DMA address.
> Using a non-canonical (e.g. encrypted or unencrypted) DMA address
> can incorrectly fail capability checks, since architecture-specific
> encryption bits are not part of the device’s actual DMA addressing
> capability. For example, arm64 adds PROT_NS_SHARED to unencrypted DMA
> addresses.
>
> Fix this by validating device DMA masks against __phys_to_dma(), ensuring
> that the architecture encryption mask does not influence the check.
>
> Fixes: b66e2ee7b6c8 ("dma: Introduce generic dma_addr_*crypted helpers")
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
> kernel/dma/direct.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 8e04f72baaa3..a5639e9415f5 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -580,12 +580,12 @@ int dma_direct_supported(struct device *dev, u64 mask)
>
> /*
> * This check needs to be against the actual bit mask value, so use
> - * phys_to_dma_unencrypted() here so that the SME encryption mask isn't
> + * __phys_to_dma() here so that the arch specific encryption mask isn't
> * part of the check.
> */
> if (IS_ENABLED(CONFIG_ZONE_DMA))
> min_mask = min_t(u64, min_mask, zone_dma_limit);
> - return mask >= phys_to_dma_unencrypted(dev, min_mask);
> + return mask >= __phys_to_dma(dev, min_mask);
This is wrong, isn't it ? For e.g., for CCA, even though the "Flag" is
added to the PA, it is really part of the actual "PA" and thus must be
checked against the full PA ?
Suzuki
> }
>
> static const struct bus_dma_region *dma_find_range(struct device *dev,
^ permalink raw reply
* Re: [PATCH] dma-direct: swiotlb: Skip encryption toggles for swiotlb allocations
From: Aneesh Kumar K.V @ 2026-01-20 9:33 UTC (permalink / raw)
To: Robin Murphy, Marek Szyprowski, iommu, linux-kernel, linux-coco
Cc: steven.price, Suzuki K Poulose, Claire Chang
In-Reply-To: <4f42d3b1-9eef-417b-9937-36578f5db6de@arm.com>
Robin Murphy <robin.murphy@arm.com> writes:
> On 19/01/2026 3:53 pm, Aneesh Kumar K.V wrote:
>> Robin Murphy <robin.murphy@arm.com> writes:
>>
>>> On 19/01/2026 9:52 am, Marek Szyprowski wrote:
>>>> On 14.01.2026 10:49, Aneesh Kumar K.V wrote:
>>>>> Aneesh Kumar K.V <aneesh.kumar@kernel.org> writes:
>>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>>> On 2026-01-09 2:51 am, Aneesh Kumar K.V wrote:
>>>>>>>> Robin Murphy <robin.murphy@arm.com> writes:
>>>>>>>>> On 2026-01-02 3:54 pm, Aneesh Kumar K.V (Arm) wrote:
>>>>>>>>>> Swiotlb backing pages are already mapped decrypted via
>>>>>>>>>> swiotlb_update_mem_attributes(), so dma-direct does not need to call
>>>>>>>>>> set_memory_decrypted() during allocation or re-encrypt the memory on
>>>>>>>>>> free.
>>>>>>>>>>
>>>>>>>>>> Handle swiotlb-backed buffers explicitly: obtain the DMA address and
>>>>>>>>>> zero the linear mapping for lowmem pages, and bypass the decrypt/encrypt
>>>>>>>>>> transitions when allocating/freeing from the swiotlb pool (detected via
>>>>>>>>>> swiotlb_find_pool()).
>>>>>>>>> swiotlb_update_mem_attributes() only applies to the default SWIOTLB
>>>>>>>>> buffer, while the dma_direct_alloc_swiotlb() path is only for private
>>>>>>>>> restricted pools (because the whole point is that restricted DMA devices
>>>>>>>>> cannot use the regular allocator/default pools). There is no redundancy
>>>>>>>>> here AFAICS.
>>>>>>>>>
>>>>>>>> But rmem_swiotlb_device_init() is also marking the entire pool decrypted
>>>>>>>>
>>>>>>>> set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>>>>>>>> rmem->size >> PAGE_SHIFT);
>>>>>>> OK, so why doesn't the commit message mention that instead of saying
>>>>>>> something which fails to justify the patch at all? ;)
>>>>>>>
>>>>>>> Furthermore, how much does this actually matter? The "real" restricted
>>>>>>> DMA use-case is on systems where dma_set_decrypted() is a no-op anyway.
>>>>>>> I know we used restricted DMA as a hack in the early days of CCA
>>>>>>> prototyping, but is it intended to actually deploy that as a supported
>>>>>>> and recommended mechanism now?
>>>>>>>
>>>>>>> Note also that the swiotlb_alloc path is essentially an emergency
>>>>>>> fallback, which doesn't work for all situations anyway - any restricted
>>>>>>> device that actually needs to make significant coherent allocations (or
>>>>>>> rather, that firmware cannot assume won't want to do so) should really
>>>>>>> have a proper coherent pool alongside its restricted one. The expected
>>>>>>> use-case here is for something like a wifi driver that only needs to
>>>>>>> allocate one or two small coherent buffers once at startup, then do
>>>>>>> everything else with streaming DMA.
>>>>>> I was aiming to bring more consistency in how swiotlb buffers are
>>>>>> handled, specifically by treating all swiotlb memory as decrypted
>>>>>> buffers, which is also how the current code behaves.
>>>>>>
>>>>>> If we are concluding that restricted DMA is not used in conjunction with
>>>>>> memory encryption, then we could, in fact, remove the
>>>>>> set_memory_decrypted() call from rmem_swiotlb_device_init() and
>>>>>> instead add failure conditions for force_dma_unencrypted(dev) in
>>>>>> is_swiotlb_for_alloc(). However, it’s worth noting that the initial
>>>>>> commit did take the memory encryption feature into account
>>>>>> (0b84e4f8b793eb4045fd64f6f514165a7974cd16).
>>>>>>
>>>>>> Please let me know if you think this needs to be fixed.
>>>>> Something like.
>>>>>
>>>>> dma-direct: restricted-dma: Do not mark the restricted DMA pool unencrypted
>>>>>
>>>>> As per commit f4111e39a52a ("swiotlb: Add restricted DMA alloc/free
>>>>> support"), the restricted-dma-pool is used in conjunction with the
>>>>> shared-dma-pool. Since allocations from the shared-dma-pool are not
>>>>> marked unencrypted, skip marking the restricted-dma-pool as unencrypted
>>>>> as well. We do not expect systems using the restricted-dma-pool to have
>>>>> memory encryption or to run with confidential computing features enabled.
>>>>>
>>>>> If a device requires unencrypted access (force_dma_unencrypted(dev)),
>>>>> the dma-direct allocator will mark the restricted-dma-pool allocation as
>>>>> unencrypted.
>>>>>
>>>>> The only disadvantage is that, when running on a CC guest with a
>>>>> different hypervisor page size, restricted-dma-pool allocation sizes
>>>>> must now be aligned to the hypervisor page size. This alignment would
>>>>> not be required if the entire pool were marked unencrypted. However, the
>>>>> new code enables the use of the restricted-dma-pool for trusted devices.
>>>>> Previously, because the entire pool was marked unencrypted, trusted
>>>>> devices were unable to allocate from it.
>>>>>
>>>>> There is still an open question regarding allocations from the
>>>>> shared-dma-pool. Currently, they are not marked unencrypted.
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>>>>>
>>>>> 1 file changed, 2 deletions(-)
>>>>> kernel/dma/swiotlb.c | 2 --
>>>>>
>>>>> modified kernel/dma/swiotlb.c
>>>>> @@ -1835,8 +1835,6 @@ static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
>>>>> return -ENOMEM;
>>>>> }
>>>>>
>>>>> - set_memory_decrypted((unsigned long)phys_to_virt(rmem->base),
>>>>> - rmem->size >> PAGE_SHIFT);
>>>>> swiotlb_init_io_tlb_pool(pool, rmem->base, nslabs,
>>>>> false, nareas);
>>>>> mem->force_bounce = true;
>>>>
>>>> Robin, could You review this? Is it ready for applying?
>>>
>>> But wouldn't this break the actual intended use of restricted pools for
>>> streaming DMA bouncing, which does depend on the buffer being
>>> pre-decrypted/shared? (Since streaming DMA mappings definitely need to
>>> be supported in nowait contexts)
>>>
>>
>> Only if we are using a restricted pool with encrypted memory.
>>
>> If we assume that swiotlb bounce buffers are always decrypted, then
>> allocations from that pool can safely skip the decrypt/encrypt
>> transitions. However, we still need to address coherent allocations via
>> the shared-dma-pool, which are explicitly marked as unencrypted.
>>
>> Given this, I’m wondering whether the best approach is to revisit the
>> original patch I posted, which moved swiotlb allocations out of
>> __dma_direct_alloc_pages(). With that separation in place, we could then
>> fix up dma_alloc_from_dev_coherent() accordingly.
>>
>> If the conclusion is that systems with encrypted memory will, in
>> practice, never use restricted-dma-pool or shared-dma-pool, then we can
>> take this patch?
>
> But if the conclusion is that it doesn't matter then that can only mean
> we don't need this patch either.
>
> We've identified that the combination of restricted DMA and a
> "meaningful" memory encryption API is theoretically slightly broken and
> can't ever have worked properly, so how do we benefit from churning it
> to just be theoretically more broken in a different way?
>
I think we should go with the original patch. I do not see it as more
broken. It is based on the simple assumption that swiotlb buffers are
always decrypted. Based on that assumption, it moves the allocation from
swiotlb out of __dma_direct_alloc_pages() and avoids the decrypt/encrypt
transition for swiotlb buffers.
For coherent allocations from the shared-dma-pool, we need to ensure
that every architecture does the right thing. For Arm CCA, we use
ioremap, which should mark the memory as decrypted/shared.
When we add trusted device support, we must also ensure that such a pool
is never attached to a trusted device. Therefore, there should never be
a need to mark that memory as encrypted/private.
-aneesh
^ permalink raw reply
* Re: [PATCH 2/2] dma-direct: Make phys_to_dma() pick encrypted vs unencrypted per device
From: kernel test robot @ 2026-01-20 9:33 UTC (permalink / raw)
To: Aneesh Kumar K.V (Arm), linux-kernel, iommu, linux-coco
Cc: llvm, oe-kbuild-all, Catalin Marinas, will, robin.murphy,
suzuki.poulose, jgg, steven.price, Marek Szyprowski,
Aneesh Kumar K.V (Arm)
In-Reply-To: <20260120064255.179425-2-aneesh.kumar@kernel.org>
Hi Aneesh,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.19-rc6 next-20260119]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Aneesh-Kumar-K-V-Arm/dma-direct-Make-phys_to_dma-pick-encrypted-vs-unencrypted-per-device/20260120-145025
base: linus/master
patch link: https://lore.kernel.org/r/20260120064255.179425-2-aneesh.kumar%40kernel.org
patch subject: [PATCH 2/2] dma-direct: Make phys_to_dma() pick encrypted vs unencrypted per device
config: arm-allnoconfig (https://download.01.org/0day-ci/archive/20260120/202601201747.22JsrCCp-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 9b8addffa70cee5b2acc5454712d9cf78ce45710)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260120/202601201747.22JsrCCp-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601201747.22JsrCCp-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from kernel/dma/mapping.c:19:
In file included from kernel/dma/direct.h:10:
>> include/linux/dma-direct.h:103:6: error: call to undeclared function 'force_dma_unencrypted'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
103 | if (force_dma_unencrypted(dev))
| ^
include/linux/dma-direct.h:103:6: note: did you mean 'phys_to_dma_unencrypted'?
include/linux/dma-direct.h:88:26: note: 'phys_to_dma_unencrypted' declared here
88 | static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
| ^
89 | phys_addr_t paddr)
90 | {
91 | return dma_addr_unencrypted(__phys_to_dma(dev, paddr));
92 | }
93 |
94 | static inline dma_addr_t phys_to_dma_encrypted(struct device *dev,
95 | phys_addr_t paddr)
96 | {
97 | return dma_addr_encrypted(__phys_to_dma(dev, paddr));
98 | }
99 |
100 | static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
101 | {
102 |
103 | if (force_dma_unencrypted(dev))
| ~~~~~~~~~~~~~~~~~~~~~
| phys_to_dma_unencrypted
>> include/linux/dma-direct.h:125:20: error: conflicting types for 'force_dma_unencrypted'
125 | static inline bool force_dma_unencrypted(struct device *dev)
| ^
include/linux/dma-direct.h:103:6: note: previous implicit declaration is here
103 | if (force_dma_unencrypted(dev))
| ^
2 errors generated.
vim +/force_dma_unencrypted +103 include/linux/dma-direct.h
102
> 103 if (force_dma_unencrypted(dev))
104 return phys_to_dma_unencrypted(dev, paddr);
105 return phys_to_dma_encrypted(dev, paddr);
106 }
107
108 static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
109 {
110 phys_addr_t paddr;
111
112 dma_addr = dma_addr_canonical(dma_addr);
113 if (dev->dma_range_map)
114 paddr = translate_dma_to_phys(dev, dma_addr);
115 else
116 paddr = dma_addr;
117
118 return paddr;
119 }
120 #endif /* !CONFIG_ARCH_HAS_PHYS_TO_DMA */
121
122 #ifdef CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED
123 bool force_dma_unencrypted(struct device *dev);
124 #else
> 125 static inline bool force_dma_unencrypted(struct device *dev)
126 {
127 return false;
128 }
129 #endif /* CONFIG_ARCH_HAS_FORCE_DMA_UNENCRYPTED */
130
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 11/16] KVM: TDX: Add x86 ops for external spt cache
From: Yan Zhao @ 2026-01-20 9:18 UTC (permalink / raw)
To: Huang, Kai
Cc: seanjc@google.com, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Li, Xiaoyao, Hansen, Dave, Wu, Binbin,
kas@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
Annapurve, Vishal, Gao, Chao, Edgecombe, Rick P, bp@alien8.de,
x86@kernel.org
In-Reply-To: <ecf01cf908570ca0a7b2d2fc712dc96146f48571.camel@intel.com>
On Tue, Jan 20, 2026 at 04:42:37PM +0800, Huang, Kai wrote:
> On Mon, 2026-01-19 at 10:31 +0800, Yan Zhao wrote:
> > On Fri, Jan 16, 2026 at 04:53:57PM -0800, Sean Christopherson wrote:
> > > On Thu, Nov 20, 2025, Rick Edgecombe wrote:
> > > > Move mmu_external_spt_cache behind x86 ops.
> > > >
> > > > In the mirror/external MMU concept, the KVM MMU manages a non-active EPT
> > > > tree for private memory (the mirror). The actual active EPT tree the
> > > > private memory is protected inside the TDX module. Whenever the mirror EPT
> > > > is changed, it needs to call out into one of a set of x86 opts that
> > > > implement various update operation with TDX specific SEAMCALLs and other
> > > > tricks. These implementations operate on the TDX S-EPT (the external).
> > > >
> > > > In reality these external operations are designed narrowly with respect to
> > > > TDX particulars. On the surface, what TDX specific things are happening to
> > > > fulfill these update operations are mostly hidden from the MMU, but there
> > > > is one particular area of interest where some details leak through.
> > > >
> > > > The S-EPT needs pages to use for the S-EPT page tables. These page tables
> > > > need to be allocated before taking the mmu lock, like all the rest. So the
> > > > KVM MMU pre-allocates pages for TDX to use for the S-EPT in the same place
> > > > where it pre-allocates the other page tables. It’s not too bad and fits
> > > > nicely with the others.
> > > >
> > > > However, Dynamic PAMT will need even more pages for the same operations.
> > > > Further, these pages will need to be handed to the arch/x86 side which used
> > > > them for DPAMT updates, which is hard for the existing KVM based cache.
> > > > The details living in core MMU code start to add up.
> > > >
> > > > So in preparation to make it more complicated, move the external page
> > > > table cache into TDX code by putting it behind some x86 ops. Have one for
> > > > topping up and one for allocation. Don’t go so far to try to hide the
> > > > existence of external page tables completely from the generic MMU, as they
> > > > are currently stored in their mirror struct kvm_mmu_page and it’s quite
> > > > handy.
> > > >
> > > > To plumb the memory cache operations through tdx.c, export some of
> > > > the functions temporarily. This will be removed in future changes.
> > > >
> > > > Acked-by: Kiryl Shutsemau <kas@kernel.org>
> > > > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > > > ---
> > >
> > > NAK. I kinda sorta get why you did this? But the pages KVM uses for page tables
> > > are KVM's, not to be mixed with PAMT pages.
> > >
> > > Eww. Definitely a hard "no". In tdp_mmu_alloc_sp_for_split(), the allocation
> > > comes from KVM:
> > >
> > > if (mirror) {
> > > sp->external_spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> > > if (!sp->external_spt) {
> > > free_page((unsigned long)sp->spt);
> > > kmem_cache_free(mmu_page_header_cache, sp);
> > > return NULL;
> > > }
> > > }
> > >
> > > But then in kvm_tdp_mmu_map(), via kvm_mmu_alloc_external_spt(), the allocation
> > > comes from get_tdx_prealloc_page()
> > >
> > > static void *tdx_alloc_external_fault_cache(struct kvm_vcpu *vcpu)
> > > {
> > > struct page *page = get_tdx_prealloc_page(&to_tdx(vcpu)->prealloc);
> > >
> > > if (WARN_ON_ONCE(!page))
> > > return (void *)__get_free_page(GFP_ATOMIC | __GFP_ACCOUNT);
> > >
> > > return page_address(page);
> > > }
> > >
> > > But then regardles of where the page came from, KVM frees it. Seriously.
> > >
> > > static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> > > {
> > > free_page((unsigned long)sp->external_spt); <=====
> > > free_page((unsigned long)sp->spt);
> > > kmem_cache_free(mmu_page_header_cache, sp);
> > > }
> > IMHO, it's by design. I don't see a problem with KVM freeing the sp->external_spt,
> > regardless of whether it's from:
> > (1) KVM's mmu cache,
> > (2) tdp_mmu_alloc_sp_for_split(), or
> > (3) tdx_alloc_external_fault_cache().
> > Please correct me if I missed anything.
> >
> > None of (1)-(3) keeps the pages in list after KVM obtains the pages and maps
> > them into SPTEs.
> >
> > So, with SPTEs as the pages' sole consumer, it's perfectly fine for KVM to free
> > the pages when freeing SPTEs. No?
> >
> > Also, in the current upstream code, after tdp_mmu_split_huge_pages_root() is
> > invoked for dirty tracking, some sp->spt are allocated from
> > tdp_mmu_alloc_sp_for_split(), while others are from kvm_mmu_memory_cache_alloc().
> > However, tdp_mmu_free_sp() can still free them without any problem.
> >
> >
>
> Well I think it's for consistency, and IMHO you can even argue this is a
> bug in the current code, because IIUC there's indeed one issue in the
> current code.
>
> When sp->spt is allocated via per-vCPU mmu_shadow_page_cache, it is
> actually initialized to SHADOW_NONPRESENT_VALUE:
>
> vcpu->arch.mmu_shadow_page_cache.init_value =
> SHADOW_NONPRESENT_VALUE;
>
> So the way sp->spt is allocated in tdp_mmu_alloc_sp_for_split() is
> actually broken IMHO because entries in sp->spt is never initialized.
The sp->spt allocated in tdp_mmu_alloc_sp_for_split() is initialized in
tdp_mmu_split_huge_page()...
> Fortunately tdp_mmu_alloc_sp_for_split() isn't reachable for TDX guests,
> so we are lucky so far.
>
> A per-VM cache requires more code to handle, but to me I still think we
> should just use the same way to allocate staff when possible, and that
> includes spt->external_spt.
^ permalink raw reply
* Re: [PATCH v4 11/16] KVM: TDX: Add x86 ops for external spt cache
From: Huang, Kai @ 2026-01-20 8:42 UTC (permalink / raw)
To: seanjc@google.com, Zhao, Yan Y
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, Wu, Binbin, kas@kernel.org,
linux-kernel@vger.kernel.org, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
Annapurve, Vishal, Gao, Chao, Edgecombe, Rick P, bp@alien8.de,
x86@kernel.org
In-Reply-To: <aW2XfpmV7FqO2HpA@yzhao56-desk.sh.intel.com>
On Mon, 2026-01-19 at 10:31 +0800, Yan Zhao wrote:
> On Fri, Jan 16, 2026 at 04:53:57PM -0800, Sean Christopherson wrote:
> > On Thu, Nov 20, 2025, Rick Edgecombe wrote:
> > > Move mmu_external_spt_cache behind x86 ops.
> > >
> > > In the mirror/external MMU concept, the KVM MMU manages a non-active EPT
> > > tree for private memory (the mirror). The actual active EPT tree the
> > > private memory is protected inside the TDX module. Whenever the mirror EPT
> > > is changed, it needs to call out into one of a set of x86 opts that
> > > implement various update operation with TDX specific SEAMCALLs and other
> > > tricks. These implementations operate on the TDX S-EPT (the external).
> > >
> > > In reality these external operations are designed narrowly with respect to
> > > TDX particulars. On the surface, what TDX specific things are happening to
> > > fulfill these update operations are mostly hidden from the MMU, but there
> > > is one particular area of interest where some details leak through.
> > >
> > > The S-EPT needs pages to use for the S-EPT page tables. These page tables
> > > need to be allocated before taking the mmu lock, like all the rest. So the
> > > KVM MMU pre-allocates pages for TDX to use for the S-EPT in the same place
> > > where it pre-allocates the other page tables. It’s not too bad and fits
> > > nicely with the others.
> > >
> > > However, Dynamic PAMT will need even more pages for the same operations.
> > > Further, these pages will need to be handed to the arch/x86 side which used
> > > them for DPAMT updates, which is hard for the existing KVM based cache.
> > > The details living in core MMU code start to add up.
> > >
> > > So in preparation to make it more complicated, move the external page
> > > table cache into TDX code by putting it behind some x86 ops. Have one for
> > > topping up and one for allocation. Don’t go so far to try to hide the
> > > existence of external page tables completely from the generic MMU, as they
> > > are currently stored in their mirror struct kvm_mmu_page and it’s quite
> > > handy.
> > >
> > > To plumb the memory cache operations through tdx.c, export some of
> > > the functions temporarily. This will be removed in future changes.
> > >
> > > Acked-by: Kiryl Shutsemau <kas@kernel.org>
> > > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > > ---
> >
> > NAK. I kinda sorta get why you did this? But the pages KVM uses for page tables
> > are KVM's, not to be mixed with PAMT pages.
> >
> > Eww. Definitely a hard "no". In tdp_mmu_alloc_sp_for_split(), the allocation
> > comes from KVM:
> >
> > if (mirror) {
> > sp->external_spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> > if (!sp->external_spt) {
> > free_page((unsigned long)sp->spt);
> > kmem_cache_free(mmu_page_header_cache, sp);
> > return NULL;
> > }
> > }
> >
> > But then in kvm_tdp_mmu_map(), via kvm_mmu_alloc_external_spt(), the allocation
> > comes from get_tdx_prealloc_page()
> >
> > static void *tdx_alloc_external_fault_cache(struct kvm_vcpu *vcpu)
> > {
> > struct page *page = get_tdx_prealloc_page(&to_tdx(vcpu)->prealloc);
> >
> > if (WARN_ON_ONCE(!page))
> > return (void *)__get_free_page(GFP_ATOMIC | __GFP_ACCOUNT);
> >
> > return page_address(page);
> > }
> >
> > But then regardles of where the page came from, KVM frees it. Seriously.
> >
> > static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> > {
> > free_page((unsigned long)sp->external_spt); <=====
> > free_page((unsigned long)sp->spt);
> > kmem_cache_free(mmu_page_header_cache, sp);
> > }
> IMHO, it's by design. I don't see a problem with KVM freeing the sp->external_spt,
> regardless of whether it's from:
> (1) KVM's mmu cache,
> (2) tdp_mmu_alloc_sp_for_split(), or
> (3) tdx_alloc_external_fault_cache().
> Please correct me if I missed anything.
>
> None of (1)-(3) keeps the pages in list after KVM obtains the pages and maps
> them into SPTEs.
>
> So, with SPTEs as the pages' sole consumer, it's perfectly fine for KVM to free
> the pages when freeing SPTEs. No?
>
> Also, in the current upstream code, after tdp_mmu_split_huge_pages_root() is
> invoked for dirty tracking, some sp->spt are allocated from
> tdp_mmu_alloc_sp_for_split(), while others are from kvm_mmu_memory_cache_alloc().
> However, tdp_mmu_free_sp() can still free them without any problem.
>
>
Well I think it's for consistency, and IMHO you can even argue this is a
bug in the current code, because IIUC there's indeed one issue in the
current code.
When sp->spt is allocated via per-vCPU mmu_shadow_page_cache, it is
actually initialized to SHADOW_NONPRESENT_VALUE:
vcpu->arch.mmu_shadow_page_cache.init_value =
SHADOW_NONPRESENT_VALUE;
So the way sp->spt is allocated in tdp_mmu_alloc_sp_for_split() is
actually broken IMHO because entries in sp->spt is never initialized.
Fortunately tdp_mmu_alloc_sp_for_split() isn't reachable for TDX guests,
so we are lucky so far.
A per-VM cache requires more code to handle, but to me I still think we
should just use the same way to allocate staff when possible, and that
includes spt->external_spt.
^ permalink raw reply
* Re: [PATCH v4 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages
From: Huang, Kai @ 2026-01-20 8:01 UTC (permalink / raw)
To: Zhao, Yan Y
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, Wu, Binbin, kas@kernel.org,
binbin.wu@linux.intel.com, mingo@redhat.com, pbonzini@redhat.com,
seanjc@google.com, Yamahata, Isaku, linux-kernel@vger.kernel.org,
tglx@linutronix.de, Annapurve, Vishal, bp@alien8.de,
Edgecombe, Rick P, Gao, Chao, x86@kernel.org
In-Reply-To: <aW8yuEX486oJ+zOp@yzhao56-desk.sh.intel.com>
>
> But in the worst-case conditions, we actually need that many.
>
> In the end, the unused pages in cache will be freed by mmu_free_memory_caches().
>
> > And AFAICT unfortunately there's no way to resolve this, unless we use
> So, I don't think it's a problem.
>
> And I agree with Binbin :)
>
Right. Overcharging is not an issue, on the contrary, we need to make
sure there's enough pages so we really need to consider the worst case.
I am not sure what I was thinking :-)
^ permalink raw reply
* Re: [PATCH v4 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages
From: Yan Zhao @ 2026-01-20 7:46 UTC (permalink / raw)
To: Huang, Kai
Cc: binbin.wu@linux.intel.com, Edgecombe, Rick P, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Li, Xiaoyao, Hansen, Dave, Wu, Binbin,
kas@kernel.org, seanjc@google.com, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
linux-kernel@vger.kernel.org, Annapurve, Vishal, bp@alien8.de,
Gao, Chao, x86@kernel.org
In-Reply-To: <a99ec2d41087c65e6b55ac53af8dc158ec5dc059.camel@intel.com>
On Tue, Jan 20, 2026 at 03:10:59PM +0800, Huang, Kai wrote:
> On Thu, 2025-11-27 at 10:38 +0800, Binbin Wu wrote:
> >
> > On 11/27/2025 6:33 AM, Edgecombe, Rick P wrote:
> > > > >
> > > > > static int tdx_topup_external_fault_cache(struct kvm_vcpu *vcpu, unsigned int cnt)
> > > > > {
> > > > > - struct vcpu_tdx *tdx = to_tdx(vcpu);
> > > > > + struct tdx_prealloc *prealloc = &to_tdx(vcpu)->prealloc;
> > > > > + int min_fault_cache_size;
> > > > >
> > > > > - return kvm_mmu_topup_memory_cache(&tdx->mmu_external_spt_cache, cnt);
> > > > > + /* External page tables */
> > > > > + min_fault_cache_size = cnt;
> > > > > + /* Dynamic PAMT pages (if enabled) */
> > > > > + min_fault_cache_size += tdx_dpamt_entry_pages() * PT64_ROOT_MAX_LEVEL;
> > > > Is the value PT64_ROOT_MAX_LEVEL intended, since dynamic PAMT pages are only
> > > > needed for 4KB level?
> > > I'm not sure I follow. We need DPAMT backing for each S-EPT page table.
> > Oh, right!
> >
> > IIUIC, PT64_ROOT_MAX_LEVEL is actually
> > - PT64_ROOT_MAX_LEVEL - 1 for S-ETP pages since root page is not needed.
> > - 1 for TD private memory page
> >
> > It's better to add a comment about it.
> >
>
> But theoretically we don't need a pair of DPAMT pages for one 4K S-EPT
> page -- we only need a pair for a entire 2M range. If these S-EPT pages
> in the fault path are allocated from the same 2M range, we are actually
> over allocating DPAMT pages.
topup() always ensures that the min page count in cache is the max count of
pages a fault needs.
For example, mmu_topup_memory_caches() ensures there are at least
PT64_ROOT_MAX_LEVEL pages in mmu_page_header_cache, which are not always
consumed by each fault.
But in the worst-case conditions, we actually need that many.
In the end, the unused pages in cache will be freed by mmu_free_memory_caches().
> And AFAICT unfortunately there's no way to resolve this, unless we use
So, I don't think it's a problem.
And I agree with Binbin :)
> tdx_alloc_page() for S-EPT pages.
^ permalink raw reply
* Re: [PATCH v4 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages
From: Huang, Kai @ 2026-01-20 7:10 UTC (permalink / raw)
To: binbin.wu@linux.intel.com, Edgecombe, Rick P
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, Zhao, Yan Y, Wu, Binbin, kas@kernel.org,
seanjc@google.com, mingo@redhat.com, pbonzini@redhat.com,
tglx@linutronix.de, Yamahata, Isaku, linux-kernel@vger.kernel.org,
Annapurve, Vishal, bp@alien8.de, Gao, Chao, x86@kernel.org
In-Reply-To: <9dcaa60c-6ffa-4f94-b002-3510110782dd@linux.intel.com>
On Thu, 2025-11-27 at 10:38 +0800, Binbin Wu wrote:
>
> On 11/27/2025 6:33 AM, Edgecombe, Rick P wrote:
> > > >
> > > > static int tdx_topup_external_fault_cache(struct kvm_vcpu *vcpu, unsigned int cnt)
> > > > {
> > > > - struct vcpu_tdx *tdx = to_tdx(vcpu);
> > > > + struct tdx_prealloc *prealloc = &to_tdx(vcpu)->prealloc;
> > > > + int min_fault_cache_size;
> > > >
> > > > - return kvm_mmu_topup_memory_cache(&tdx->mmu_external_spt_cache, cnt);
> > > > + /* External page tables */
> > > > + min_fault_cache_size = cnt;
> > > > + /* Dynamic PAMT pages (if enabled) */
> > > > + min_fault_cache_size += tdx_dpamt_entry_pages() * PT64_ROOT_MAX_LEVEL;
> > > Is the value PT64_ROOT_MAX_LEVEL intended, since dynamic PAMT pages are only
> > > needed for 4KB level?
> > I'm not sure I follow. We need DPAMT backing for each S-EPT page table.
> Oh, right!
>
> IIUIC, PT64_ROOT_MAX_LEVEL is actually
> - PT64_ROOT_MAX_LEVEL - 1 for S-ETP pages since root page is not needed.
> - 1 for TD private memory page
>
> It's better to add a comment about it.
>
But theoretically we don't need a pair of DPAMT pages for one 4K S-EPT
page -- we only need a pair for a entire 2M range. If these S-EPT pages
in the fault path are allocated from the same 2M range, we are actually
over allocating DPAMT pages.
And AFAICT unfortunately there's no way to resolve this, unless we use
tdx_alloc_page() for S-EPT pages.
^ permalink raw reply
* [PATCH 2/2] dma-direct: Make phys_to_dma() pick encrypted vs unencrypted per device
From: Aneesh Kumar K.V (Arm) @ 2026-01-20 6:42 UTC (permalink / raw)
To: linux-kernel, iommu, linux-coco
Cc: Catalin Marinas, will, robin.murphy, suzuki.poulose, jgg,
steven.price, Marek Szyprowski, Aneesh Kumar K.V (Arm)
In-Reply-To: <20260120064255.179425-1-aneesh.kumar@kernel.org>
On systems that apply an address encryption tag/mask to DMA addresses, the
choice of encrypted vs unencrypted DMA address is device-dependent (e.g.
TDISP trusted devices vs non-trusted devices).
Teach phys_to_dma() to make this choice based on
force_dma_unencrypted(dev), and convert dma-direct users to call
phys_to_dma() directly. With this in place, drop phys_to_dma_direct() as
redundant.
Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
---
include/linux/dma-direct.h | 18 +++++++++++-------
kernel/dma/direct.c | 20 ++++++--------------
2 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index c249912456f9..e2e3a08373a1 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -90,17 +90,21 @@ static inline dma_addr_t phys_to_dma_unencrypted(struct device *dev,
{
return dma_addr_unencrypted(__phys_to_dma(dev, paddr));
}
-/*
- * If memory encryption is supported, phys_to_dma will set the memory encryption
- * bit in the DMA address, and dma_to_phys will clear it.
- * phys_to_dma_unencrypted is for use on special unencrypted memory like swiotlb
- * buffers.
- */
-static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
+
+static inline dma_addr_t phys_to_dma_encrypted(struct device *dev,
+ phys_addr_t paddr)
{
return dma_addr_encrypted(__phys_to_dma(dev, paddr));
}
+static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
+{
+
+ if (force_dma_unencrypted(dev))
+ return phys_to_dma_unencrypted(dev, paddr);
+ return phys_to_dma_encrypted(dev, paddr);
+}
+
static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dma_addr)
{
phys_addr_t paddr;
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index a5639e9415f5..59d7d9e15e17 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -23,14 +23,6 @@
*/
u64 zone_dma_limit __ro_after_init = DMA_BIT_MASK(24);
-static inline dma_addr_t phys_to_dma_direct(struct device *dev,
- phys_addr_t phys)
-{
- if (force_dma_unencrypted(dev))
- return phys_to_dma_unencrypted(dev, phys);
- return phys_to_dma(dev, phys);
-}
-
static inline struct page *dma_direct_to_page(struct device *dev,
dma_addr_t dma_addr)
{
@@ -40,7 +32,7 @@ static inline struct page *dma_direct_to_page(struct device *dev,
u64 dma_direct_get_required_mask(struct device *dev)
{
phys_addr_t phys = (phys_addr_t)(max_pfn - 1) << PAGE_SHIFT;
- u64 max_dma = phys_to_dma_direct(dev, phys);
+ u64 max_dma = phys_to_dma(dev, phys);
return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
}
@@ -69,7 +61,7 @@ static gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 *phys_limit)
bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
{
- dma_addr_t dma_addr = phys_to_dma_direct(dev, phys);
+ dma_addr_t dma_addr = phys_to_dma(dev, phys);
if (dma_addr == DMA_MAPPING_ERROR)
return false;
@@ -178,7 +170,7 @@ static void *dma_direct_alloc_from_pool(struct device *dev, size_t size,
page = dma_alloc_from_pool(dev, size, &ret, gfp, dma_coherent_ok);
if (!page)
return NULL;
- *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+ *dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
}
@@ -196,7 +188,7 @@ static void *dma_direct_alloc_no_mapping(struct device *dev, size_t size,
arch_dma_prep_coherent(page, size);
/* return the page pointer as the opaque cookie */
- *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+ *dma_handle = phys_to_dma(dev, page_to_phys(page));
return page;
}
@@ -311,7 +303,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
goto out_encrypt_pages;
}
- *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+ *dma_handle = phys_to_dma(dev, page_to_phys(page));
return ret;
out_encrypt_pages:
@@ -392,7 +384,7 @@ struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
if (dma_set_decrypted(dev, ret, size))
goto out_leak_pages;
memset(ret, 0, size);
- *dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
+ *dma_handle = phys_to_dma(dev, page_to_phys(page));
return page;
out_leak_pages:
return NULL;
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox