[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver


  • To: Julien Grall <julien@xxxxxxx>
  • From: Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>
  • Date: Wed, 19 Jan 2022 10:37:15 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=DAfxaFcPwTSEPGACiGT0lrm3EISMJk4FBhWpufo50gs=; b=SuiKNICzkhOLNM7zETgGHU4XAGiNr9oXf796EJzWipKBL+xQNuNRAJYOFYHFlFpxsDu2m7UGm4xrsFNRaNStbqtMQh7cA+F6v79ZuZB+Y7Ke0H2uifuNodeHOtirS4HZzDejbyBsUvutwGGSBrQ1yfepCgmQLd9f0HG5zYcnYsUETEHBPKOkA8eA00pZwrPRPFAx6whAN4+RjjqktEp4L8PB7qCwwY47EMJbvWbl2InoxhFAf8FZX8W9NruaUBc/e6hyLQEE/5UFw56yaAH4Yr6t+acGSN1mAC7xTmVLmnAb2E7e+jl33eO9ymP/m+5sCSEnPsPm78KQYhpDohqaiw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VY/BKyMJVynwgVl9uT9kxe7z5GlK6dditXSm9nfjb6HrJOppUmR+bgKQO05ZnJFJnloucAaewU4edER5T511KCNUigkir7vmO6t0YHsDm5kKeuEpG9O7ZVMCH1Kf7cI7giOoiH+5rUQumuyAHAcNvibG/gikLequ5KmZJFFo/JyJ6voU+R8sh18m+mLMZTwVglcSnd4MvOuUVwrzgv8xJEZiH3nGJxtWQ/rTctvFVeaNMO1ZzIGcprmaW5asalc77LVvezmt/gfwsotXfEXFgArQcqWNTpQf8MQ4k0PpkR3mZVWRWXFFWCKacV67L9/WB8T0+BHfrPNOsdjmbFEvug==
  • Cc: Oleksandr <olekstysh@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>
  • Delivery-date: Wed, 19 Jan 2022 10:37:47 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHX8M3JF7Ng56/tV0+8/7pODiaWfKw2ks6AgAAeBwCAAAQPgIAABfaAgAAsl4CABKcNgIAGOOkAgAAnH4CAD3d9gIAEwfEAgAACbgCAABxaAIAABdkAgBQS24A=
  • Thread-topic: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver

Hi Julien,

On Thu, Jan 06, 2022 at 04:04:34PM +0000, Julien Grall wrote:
> Hi,
> 
> On 06/01/2022 15:43, Oleksii Moisieiev wrote:
> > On Thu, Jan 06, 2022 at 02:02:10PM +0000, Julien Grall wrote:
> > > 
> > > 
> > > On 06/01/2022 13:53, Oleksii Moisieiev wrote:
> > > > Hi Julien,
> > > 
> > > Hi,
> > > 
> > > > 
> > > > On Mon, Jan 03, 2022 at 01:14:17PM +0000, Julien Grall wrote:
> > > > > Hi,
> > > > > 
> > > > > On 24/12/2021 17:02, Oleksii Moisieiev wrote:
> > > > > > On Fri, Dec 24, 2021 at 03:42:42PM +0100, Julien Grall wrote:
> > > > > > > On 20/12/2021 16:41, Oleksii Moisieiev wrote:
> > > > > > > > >       2) What are the expected memory attribute for the 
> > > > > > > > > regions?
> > > > > > > > 
> > > > > > > > xen uses iommu_permit_access to pass agent page to the guest. 
> > > > > > > > So guest can access the page directly.
> > > > > > > 
> > > > > > > I think you misunderstood my comment. Memory can be mapped with 
> > > > > > > various type
> > > > > > > (e.g. Device, Memory) and attribute (cacheable, 
> > > > > > > non-cacheable...). What will
> > > > > > > the firmware expect? What will the guest OS usually?
> > > > > > > 
> > > > > > > The reason I am asking is the attributes have to matched to avoid 
> > > > > > > any
> > > > > > > coherency issues. At the moment, if you use 
> > > > > > > XEN_DOMCTL_memory_mapping, Xen
> > > > > > > will configure the stage-2 to use Device nGnRnE. As the result, 
> > > > > > > the result
> > > > > > > memory access will be Device nGnRnE which is very strict.
> > > > > > > 
> > > > > > 
> > > > > > Let me share with you the configuration example:
> > > > > > scmi expects memory to be configured in the device-tree:
> > > > > > 
> > > > > > cpu_scp_shm: scp-shmem@0xXXXXXXX {
> > > > > >     compatible = "arm,scmi-shmem";
> > > > > >     reg = <0x0 0xXXXXXX 0x0 0x1000>;
> > > > > > };
> > > > > > 
> > > > > > where XXXXXX address I allocate in alloc_magic_pages function.
> > > > > 
> > > > > The goal of alloc_magic_pages() is to allocate RAM. However, what you 
> > > > > want
> > > > > is a guest physical address and then map a part of the shared page.
> > > > 
> > > > Do you mean that I can't use alloc_magic_pages to allocate shared
> > > > memory region for SCMI?
> > > Correct. alloc_magic_pages() will allocate a RAM page and then assign to 
> > > the
> > > guest. From your description, this is not what you want because you will
> > > call XEN_DOMCTL_memory_mapping (and therefore replace the mapping).
> > > 
> > 
> > Ok thanks, I will refactor this part in v2.
> > 
> > > > 
> > > > > 
> > > > > I can see two options here:
> > > > >     1) Hardcode the SCMI region in the memory map
> > > > >     2) Create a new region in the memory map that can be used for 
> > > > > reserving
> > > > > memory for mapping.
> > > > 
> > > > Could you please explain what do you mean under the "new region in the
> > > > memory map"?
> > > 
> > > I mean reserving some guest physical address that could be used for map 
> > > host
> > > physical address (e.g. SCMI region, GIC CPU interface...).
> > > 
> > > So rather than hardcoding the address, we have something more flexible.
> > > 
> > 
> > Ok, I will fix that in v2.
> 
> Just for avoidance of doubt. I was clarify option 2 and not requesting to
> implement. That said, if you want to implement option 2 I would be happy to
> review it.
> 

I'm thinking about the best way to reserve address for the domain.
We have xen_pfn_t shared_info_pfn in struct xc_dom_image which is not
used for Arm architecture. It can be set from shared_info_arm callback,
defined in xg_dom_arm.c.
I can use shared_info to store address of the SCMI and then use map_sci_page to
call XEN_DOMCTL_memory_mapping.

This will allow me to reuse existing functionality and do not allocate
extra RAM.

What do you think about that?

--
Best regards,
Oleksii.


> > 
> > > > 
> > > > > 
> > > > > We still have plenty of space in the guest memory map. So the former 
> > > > > is
> > > > > probably going to be fine for now.
> > > > > 
> > > > > > Then I get paddr of the scmi channel for this domain and use
> > > > > > XEN_DOMCTL_memory_mapping to map scmi channel address to gfn.
> > > > > >    > Hope I wass able to answer your question.
> > > > > 
> > > > > What you provided me is how the guest OS will locate the shared 
> > > > > memory. This
> > > > > still doesn't tell me which memory attribute will be used to map the 
> > > > > page in
> > > > > Stage-1 (guest page-tables).
> > > > > 
> > > > > To find that out, you want to look at the driver and how the mapping 
> > > > > is
> > > > > done. The Linux driver (drivers/firmware/arm_scmi) is using 
> > > > > devm_ioremap()
> > > > > (see smc_chan_setup()).
> > > > > 
> > > > > Under the hood, the function devm_ioremap() is using PROT_DEVICE_nGnRE
> > > > > (arm64) which is one of the most restrictive memory attribute.
> > > > > 
> > > > > This means the firmware should be able to deal with the most 
> > > > > restrictive
> > > > > attribute and therefore using XEN_DOMCTL_memory_mapping to map the 
> > > > > shared
> > > > > page in stage-2 should be fine.
> > > > > 
> > > > 
> > > > I'm using vmap call to map channel memory (see smc_create_channel()).
> > > > vmap call sets PAGE_HYPERVISOR flag which sets MT_NORMAL (0x7) flag.
> > > 
> > > You want to use ioremap().
> > > 
> > 
> > I've used ioremap originally, but changed it to vmap because ioremap
> > doesn't support memcpy.
> > What if I use __vmap with MT_DEVICE_nGnRE flag?
> 
> That's not going to help. Our implementation of memcpy() is using unaligned
> access (which is forbidden on Device memory).
> 
> You will need something similar to memcpy_toio() in Linux. I don't think we
> have one today in Xen, so I would suggest to import the implementation from
> Linux.
> 
> > 
> > > > Considering that protocol is synchronous and only one agent per channel 
> > > > is
> > > > expected - this works fine for now.
> > > > But I agree that the same memory attributes should be used in xen and
> > > > kernel. I fill fix that in v2.
> > > 
> > > I am a bit confused. Are you mapping the full shared memory area in Xen? 
> > > If
> > > yes, why do you need to map the memory that is going to be shared with a
> > > domain?
> > > 
> > 
> > Xen should have an access to all agent channels because it should send
> > SCMI_BASE_DISCOVER_AGENT to each channel and receive agent_id during
> > scmi_probe call.
> 
> Hmmm... Just to confirm, this will only happen during Xen boot? IOW, Xen
> will never write to the channel when a domain is running?
> 
> If yes, then I think it would be best to unmap the channel once they are
> used. This would prevent all sort of issues (e.g. Xen mistakenly written in
> them).
> 
> Cheers,
> 
> -- 
> Julien Grall


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.