Re: Reg. Tee init fail...

Re: Reg. Tee init fail...

[Julien Grall]

Hi,

(moving the discussion to xen-devel as I think it is more appropriate)

On 24/06/2022 10:53, SK, SivaSangeetha (Siva Sangeetha) wrote:
> [AMD Official Use Only - General]

Not clear what this means.

> 
> Hi Xen team,
> 
> In TEE driver, We allocate a ring buffer, get its physical address from __pa() macro, pass the physical address to secure processor for mapping it and using in secure processor side.
> 
> Source: https://elixir.bootlin.com/linux/latest/source/drivers/crypto/ccp/tee-dev.c#L132
> 
> This works good natively in Dom0 on the target.
> When we boot the same Dom0 kernel, with Xen hypervisor enabled, ring init fails.

Do you have any error message or error code?

> 
> 
> We suspect that the address passed to secure processor, is not same when xen is enabled, and when xen is enabled, some level of address translation might be required to get exact physical address.

If you are using Xen upstream, Dom0 will be mapped with IPA == PA. So 
there should be no need for translation.

Can you provide more details on your setup (version of Xen, Linux...)?

Cheers,

-- 
Julien Grall

Re: Reg. Tee init fail...

[Stefano Stabellini]

Adding Juergen and Boris because this is a Linux/x86 issue.


As you can see from this Linux driver:
https://elixir.bootlin.com/linux/latest/source/drivers/crypto/ccp/tee-dev.c#L132

Linux as dom0 on x86 is trying to communicate with firmware (TEE). Linux
is calling __pa to pass a physical address to firmware. However, __pa
returns a "fake" address not an mfn. I imagine that a quick workaround
would be to call "virt_to_machine" instead of "__pa" in tee-dev.c.

Normally, if this was a device, the "right fix" would be to use
swiotlb-xen:xen_swiotlb_map_page to get back a real physical address.

However, xen_swiotlb_map_page is meant to be used as part of the dma_ops
API and takes a struct device *dev as input parameter. Maybe
xen_swiotlb_map_page can be used for tee-dev as well?


Basically tee-dev would need to call dma_map_page before passing
addresses to firmware, and dma_unmap_page when it is done. E.g.:


  cmd_buffer = dma_map_page(dev, virt_to_page(cmd),
                            cmd & ~PAGE_MASK,
                            ring_size,
                            DMA_TO_DEVICE);


Juergen, Boris,
what do you think?



On Fri, 24 Jun 2022, Julien Grall wrote:
> Hi,
> 
> (moving the discussion to xen-devel as I think it is more appropriate)
> 
> On 24/06/2022 10:53, SK, SivaSangeetha (Siva Sangeetha) wrote:
> > [AMD Official Use Only - General]
> 
> Not clear what this means.
> 
> > 
> > Hi Xen team,
> > 
> > In TEE driver, We allocate a ring buffer, get its physical address from
> > __pa() macro, pass the physical address to secure processor for mapping it
> > and using in secure processor side.
> > 
> > Source:
> > https://elixir.bootlin.com/linux/latest/source/drivers/crypto/ccp/tee-dev.c#L132
> > 
> > This works good natively in Dom0 on the target.
> > When we boot the same Dom0 kernel, with Xen hypervisor enabled, ring init
> > fails.
> 
> Do you have any error message or error code?
> 
> > 
> > 
> > We suspect that the address passed to secure processor, is not same when xen
> > is enabled, and when xen is enabled, some level of address translation might
> > be required to get exact physical address.
> 
> If you are using Xen upstream, Dom0 will be mapped with IPA == PA. So there
> should be no need for translation.
> 
> Can you provide more details on your setup (version of Xen, Linux...)?
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

Re: Reg. Tee init fail...

[Boris Ostrovsky]

On 6/29/22 4:03 PM, Stefano Stabellini wrote:
> Adding Juergen and Boris because this is a Linux/x86 issue.
>
>
> As you can see from this Linux driver:
> https://elixir.bootlin.com/linux/latest/source/drivers/crypto/ccp/tee-dev.c#L132
>
> Linux as dom0 on x86 is trying to communicate with firmware (TEE). Linux
> is calling __pa to pass a physical address to firmware. However, __pa
> returns a "fake" address not an mfn. I imagine that a quick workaround
> would be to call "virt_to_machine" instead of "__pa" in tee-dev.c.


It's probably worth a try but it seems we may need to OR the result with C-bit (i.e. sme_me_mask). Or (for testing purposes) run with TSME on, I think C-bit is not set then.


-boris


> Normally, if this was a device, the "right fix" would be to use
> swiotlb-xen:xen_swiotlb_map_page to get back a real physical address.
>
> However, xen_swiotlb_map_page is meant to be used as part of the dma_ops
> API and takes a struct device *dev as input parameter. Maybe
> xen_swiotlb_map_page can be used for tee-dev as well?
>
>
> Basically tee-dev would need to call dma_map_page before passing
> addresses to firmware, and dma_unmap_page when it is done. E.g.:
>
>
>    cmd_buffer = dma_map_page(dev, virt_to_page(cmd),
>                              cmd & ~PAGE_MASK,
>                              ring_size,
>                              DMA_TO_DEVICE);
>
>
> Juergen, Boris,
> what do you think?
>
>
>
> On Fri, 24 Jun 2022, Julien Grall wrote:
>> Hi,
>>
>> (moving the discussion to xen-devel as I think it is more appropriate)
>>
>> On 24/06/2022 10:53, SK, SivaSangeetha (Siva Sangeetha) wrote:
>>> [AMD Official Use Only - General]
>> Not clear what this means.
>>
>>> Hi Xen team,
>>>
>>> In TEE driver, We allocate a ring buffer, get its physical address from
>>> __pa() macro, pass the physical address to secure processor for mapping it
>>> and using in secure processor side.
>>>
>>> Source:
>>> https://elixir.bootlin.com/linux/latest/source/drivers/crypto/ccp/tee-dev.c#L132
>>>
>>> This works good natively in Dom0 on the target.
>>> When we boot the same Dom0 kernel, with Xen hypervisor enabled, ring init
>>> fails.
>> Do you have any error message or error code?
>>
>>>
>>> We suspect that the address passed to secure processor, is not same when xen
>>> is enabled, and when xen is enabled, some level of address translation might
>>> be required to get exact physical address.
>> If you are using Xen upstream, Dom0 will be mapped with IPA == PA. So there
>> should be no need for translation.
>>
>> Can you provide more details on your setup (version of Xen, Linux...)?
>>
>> Cheers,
>>
>> -- 
>> Julien Grall
>>

Re: Reg. Tee init fail...

[Andrew Cooper]

On 24/06/2022 18:28, Julien Grall wrote:
> Hi,
>
> (moving the discussion to xen-devel as I think it is more appropriate)
>
> On 24/06/2022 10:53, SK, SivaSangeetha (Siva Sangeetha) wrote:
>> [AMD Official Use Only - General]
>
> Not clear what this means.

Its an Office365 thing automatically inserted post-send.  Some people,
depending on organisational policy, cannot opt out of it.

It's irritating, but best just ignored.

~Andrew

RE: Reg. Tee init fail...

[SK, SivaSangeetha (Siva Sangeetha)]

[AMD Official Use Only - General]

+team

-----Original Message-----
From: Stefano Stabellini <sstabellini@kernel.org> 
Sent: Thursday, June 30, 2022 1:34 AM
To: Julien Grall <julien@xen.org>
Cc: SK, SivaSangeetha (Siva Sangeetha) <SivaSangeetha.SK@amd.com>; xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; jgross@suse.com; boris.ostrovsky@oracle.com
Subject: Re: Reg. Tee init fail...

Adding Juergen and Boris because this is a Linux/x86 issue.


As you can see from this Linux driver:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fcrypto%2Fccp%2Ftee-dev.c%23L132&amp;data=05%7C01%7CSivaSangeetha.SK%40amd.com%7Ce962a907794f4917a80b08da5a0a7b3b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637921298315828104%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=NxmMUckiDRGLv3qLJrhZKBt2zNTuomEZqYJdV74tXxA%3D&amp;reserved=0

Linux as dom0 on x86 is trying to communicate with firmware (TEE). Linux is calling __pa to pass a physical address to firmware. However, __pa returns a "fake" address not an mfn. I imagine that a quick workaround would be to call "virt_to_machine" instead of "__pa" in tee-dev.c.

Normally, if this was a device, the "right fix" would be to use swiotlb-xen:xen_swiotlb_map_page to get back a real physical address.

However, xen_swiotlb_map_page is meant to be used as part of the dma_ops API and takes a struct device *dev as input parameter. Maybe xen_swiotlb_map_page can be used for tee-dev as well?


Basically tee-dev would need to call dma_map_page before passing addresses to firmware, and dma_unmap_page when it is done. E.g.:


  cmd_buffer = dma_map_page(dev, virt_to_page(cmd),
                            cmd & ~PAGE_MASK,
                            ring_size,
                            DMA_TO_DEVICE);


Juergen, Boris,
what do you think?



On Fri, 24 Jun 2022, Julien Grall wrote:
> Hi,
> 
> (moving the discussion to xen-devel as I think it is more appropriate)
> 
> On 24/06/2022 10:53, SK, SivaSangeetha (Siva Sangeetha) wrote:
> > [AMD Official Use Only - General]
> 
> Not clear what this means.
> 
> > 
> > Hi Xen team,
> > 
> > In TEE driver, We allocate a ring buffer, get its physical address 
> > from
> > __pa() macro, pass the physical address to secure processor for 
> > mapping it and using in secure processor side.
> > 
> > Source:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fel
> > ixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fcrypto%2Fccp%
> > 2Ftee-dev.c%23L132&amp;data=05%7C01%7CSivaSangeetha.SK%40amd.com%7Ce
> > 962a907794f4917a80b08da5a0a7b3b%7C3dd8961fe4884e608e11a82d994e183d%7
> > C0%7C0%7C637921298315828104%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> > MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&a
> > mp;sdata=NxmMUckiDRGLv3qLJrhZKBt2zNTuomEZqYJdV74tXxA%3D&amp;reserved
> > =0
> > 
> > This works good natively in Dom0 on the target.
> > When we boot the same Dom0 kernel, with Xen hypervisor enabled, ring 
> > init fails.
> 
> Do you have any error message or error code?
> 
> > 
> > 
> > We suspect that the address passed to secure processor, is not same 
> > when xen is enabled, and when xen is enabled, some level of address 
> > translation might be required to get exact physical address.
> 
> If you are using Xen upstream, Dom0 will be mapped with IPA == PA. So 
> there should be no need for translation.
> 
> Can you provide more details on your setup (version of Xen, Linux...)?
> 
> Cheers,
> 
> --
> Julien Grall
> 

Re: Reg. Tee init fail...

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 2433 bytes --]

On 30.06.22 05:32, SK, SivaSangeetha (Siva Sangeetha) wrote:
> [AMD Official Use Only - General]
> 
> +team
> 
> -----Original Message-----
> From: Stefano Stabellini <sstabellini@kernel.org>
> Sent: Thursday, June 30, 2022 1:34 AM
> To: Julien Grall <julien@xen.org>
> Cc: SK, SivaSangeetha (Siva Sangeetha) <SivaSangeetha.SK@amd.com>; xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; jgross@suse.com; boris.ostrovsky@oracle.com
> Subject: Re: Reg. Tee init fail...
> 
> Adding Juergen and Boris because this is a Linux/x86 issue.
> 
> 
> As you can see from this Linux driver:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fcrypto%2Fccp%2Ftee-dev.c%23L132&amp;data=05%7C01%7CSivaSangeetha.SK%40amd.com%7Ce962a907794f4917a80b08da5a0a7b3b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637921298315828104%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=NxmMUckiDRGLv3qLJrhZKBt2zNTuomEZqYJdV74tXxA%3D&amp;reserved=0
> 
> Linux as dom0 on x86 is trying to communicate with firmware (TEE). Linux is calling __pa to pass a physical address to firmware. However, __pa returns a "fake" address not an mfn. I imagine that a quick workaround would be to call "virt_to_machine" instead of "__pa" in tee-dev.c.
> 
> Normally, if this was a device, the "right fix" would be to use swiotlb-xen:xen_swiotlb_map_page to get back a real physical address.
> 
> However, xen_swiotlb_map_page is meant to be used as part of the dma_ops API and takes a struct device *dev as input parameter. Maybe xen_swiotlb_map_page can be used for tee-dev as well?
> 
> 
> Basically tee-dev would need to call dma_map_page before passing addresses to firmware, and dma_unmap_page when it is done. E.g.:
> 
> 
>    cmd_buffer = dma_map_page(dev, virt_to_page(cmd),
>                              cmd & ~PAGE_MASK,
>                              ring_size,
>                              DMA_TO_DEVICE);
> 
> 
> Juergen, Boris,
> what do you think?

Yes, I think using the DMA interface is the correct way to handle that.

BTW, I did a similar fix for the dcdbas driver recently:

https://lore.kernel.org/r/20220318150950.16843-1-jgross@suse.com


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: Reg. Tee init fail...

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 4022 bytes --]

On Thu, Jun 30, 2022 at 07:31:38AM +0200, Juergen Gross wrote:
> On 30.06.22 05:32, SK, SivaSangeetha (Siva Sangeetha) wrote:
> > [AMD Official Use Only - General]
> > 
> > +team
> > 
> > -----Original Message-----
> > From: Stefano Stabellini <sstabellini@kernel.org>
> > Sent: Thursday, June 30, 2022 1:34 AM
> > To: Julien Grall <julien@xen.org>
> > Cc: SK, SivaSangeetha (Siva Sangeetha) <SivaSangeetha.SK@amd.com>; xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; jgross@suse.com; boris.ostrovsky@oracle.com
> > Subject: Re: Reg. Tee init fail...
> > 
> > Adding Juergen and Boris because this is a Linux/x86 issue.
> > 
> > 
> > As you can see from this Linux driver:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fcrypto%2Fccp%2Ftee-dev.c%23L132&amp;data=05%7C01%7CSivaSangeetha.SK%40amd.com%7Ce962a907794f4917a80b08da5a0a7b3b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637921298315828104%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=NxmMUckiDRGLv3qLJrhZKBt2zNTuomEZqYJdV74tXxA%3D&amp;reserved=0
> > 
> > Linux as dom0 on x86 is trying to communicate with firmware (TEE). Linux is calling __pa to pass a physical address to firmware. However, __pa returns a "fake" address not an mfn. I imagine that a quick workaround would be to call "virt_to_machine" instead of "__pa" in tee-dev.c.
> > 
> > Normally, if this was a device, the "right fix" would be to use swiotlb-xen:xen_swiotlb_map_page to get back a real physical address.
> > 
> > However, xen_swiotlb_map_page is meant to be used as part of the dma_ops API and takes a struct device *dev as input parameter. Maybe xen_swiotlb_map_page can be used for tee-dev as well?
> > 
> > 
> > Basically tee-dev would need to call dma_map_page before passing addresses to firmware, and dma_unmap_page when it is done. E.g.:
> > 
> > 
> >    cmd_buffer = dma_map_page(dev, virt_to_page(cmd),
> >                              cmd & ~PAGE_MASK,
> >                              ring_size,
> >                              DMA_TO_DEVICE);
> > 
> > 
> > Juergen, Boris,
> > what do you think?
> 
> Yes, I think using the DMA interface is the correct way to handle that.
> 
> BTW, I did a similar fix for the dcdbas driver recently:
> 
> https://lore.kernel.org/r/20220318150950.16843-1-jgross@suse.com

I hit similar issue, and the patch below made it work for me (ugly
workaround), or at least stop complaining.

But note one of those places have this comment:

	/* We need actual physical address instead of DMA address, since
	 * Trusted OS running on AMD Secure Processor will map this region
	 */

I guess it means AMD Secure Processor bypasses IOMMU...

---8<---
diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 5c9d47f3be37..9d440fc8a56d 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -15,6 +15,7 @@
 #include <linux/gfp.h>
 #include <linux/psp-sev.h>
 #include <linux/psp-tee.h>
+#include <xen/page.h>
 
 #include "psp-dev.h"
 #include "tee-dev.h"
@@ -39,7 +40,7 @@ static int tee_alloc_ring(struct psp_tee_device *tee, int ring_size)
 	memset(start_addr, 0x0, ring_size);
 	rb_mgr->ring_start = start_addr;
 	rb_mgr->ring_size = ring_size;
-	rb_mgr->ring_pa = __psp_pa(start_addr);
+	rb_mgr->ring_pa = virt_to_machine(start_addr).maddr;
 	mutex_init(&rb_mgr->mutex);
 
 	return 0;
@@ -129,7 +130,7 @@ static int tee_init_ring(struct psp_tee_device *tee)
 		return -ENOMEM;
 	}
 
-	cmd_buffer = __psp_pa((void *)cmd);
+	cmd_buffer = virt_to_machine((void *)cmd).maddr;
 
 	/* Send command buffer details to Trusted OS by writing to
 	 * CPU-PSP message registers




-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Reg. Tee init fail...

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 3326 bytes --]

On 11.08.22 00:55, Marek Marczykowski-Górecki wrote:
> On Thu, Jun 30, 2022 at 07:31:38AM +0200, Juergen Gross wrote:
>> On 30.06.22 05:32, SK, SivaSangeetha (Siva Sangeetha) wrote:
>>> [AMD Official Use Only - General]
>>>
>>> +team
>>>
>>> -----Original Message-----
>>> From: Stefano Stabellini <sstabellini@kernel.org>
>>> Sent: Thursday, June 30, 2022 1:34 AM
>>> To: Julien Grall <julien@xen.org>
>>> Cc: SK, SivaSangeetha (Siva Sangeetha) <SivaSangeetha.SK@amd.com>; xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; jgross@suse.com; boris.ostrovsky@oracle.com
>>> Subject: Re: Reg. Tee init fail...
>>>
>>> Adding Juergen and Boris because this is a Linux/x86 issue.
>>>
>>>
>>> As you can see from this Linux driver:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fcrypto%2Fccp%2Ftee-dev.c%23L132&amp;data=05%7C01%7CSivaSangeetha.SK%40amd.com%7Ce962a907794f4917a80b08da5a0a7b3b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637921298315828104%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=NxmMUckiDRGLv3qLJrhZKBt2zNTuomEZqYJdV74tXxA%3D&amp;reserved=0
>>>
>>> Linux as dom0 on x86 is trying to communicate with firmware (TEE). Linux is calling __pa to pass a physical address to firmware. However, __pa returns a "fake" address not an mfn. I imagine that a quick workaround would be to call "virt_to_machine" instead of "__pa" in tee-dev.c.
>>>
>>> Normally, if this was a device, the "right fix" would be to use swiotlb-xen:xen_swiotlb_map_page to get back a real physical address.
>>>
>>> However, xen_swiotlb_map_page is meant to be used as part of the dma_ops API and takes a struct device *dev as input parameter. Maybe xen_swiotlb_map_page can be used for tee-dev as well?
>>>
>>>
>>> Basically tee-dev would need to call dma_map_page before passing addresses to firmware, and dma_unmap_page when it is done. E.g.:
>>>
>>>
>>>     cmd_buffer = dma_map_page(dev, virt_to_page(cmd),
>>>                               cmd & ~PAGE_MASK,
>>>                               ring_size,
>>>                               DMA_TO_DEVICE);
>>>
>>>
>>> Juergen, Boris,
>>> what do you think?
>>
>> Yes, I think using the DMA interface is the correct way to handle that.
>>
>> BTW, I did a similar fix for the dcdbas driver recently:
>>
>> https://lore.kernel.org/r/20220318150950.16843-1-jgross@suse.com
> 
> I hit similar issue, and the patch below made it work for me (ugly
> workaround), or at least stop complaining.
> 
> But note one of those places have this comment:
> 
> 	/* We need actual physical address instead of DMA address, since
> 	 * Trusted OS running on AMD Secure Processor will map this region
> 	 */
> 
> I guess it means AMD Secure Processor bypasses IOMMU...

It seems not to be attached via an IOMMU, same as for the DCDBAS fix,
where the address is used in SMM handler running on the main processor.

It should be possible to use DMA mapping in your case, too, as long as
the related device has the correct DMA settings not using any IOMMU
translations.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]