[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC v1 3/5] xen/arm: introduce SCMI-SMC mediator driver
On Fri, Dec 24, 2021 at 03:42:42PM +0100, Julien Grall wrote: > On 20/12/2021 16:41, Oleksii Moisieiev wrote: > > Hi Julien, > > Hello, > > > On Fri, Dec 17, 2021 at 04:38:31PM +0000, Julien Grall wrote: > > > > > > > > > On 17/12/2021 13:58, Oleksii Moisieiev wrote: > > > > Hi Julien, > > > > > > Hi, > > > > > > > On Fri, Dec 17, 2021 at 01:37:35PM +0000, Julien Grall wrote: > > > > > Hi, > > > > > > > > > > On 17/12/2021 13:23, Oleksii Moisieiev wrote: > > > > > > > > +static int map_memory_to_domain(struct domain *d, uint64_t > > > > > > > > addr, uint64_t len) > > > > > > > > +{ > > > > > > > > + return iomem_permit_access(d, paddr_to_pfn(addr), > > > > > > > > + paddr_to_pfn(PAGE_ALIGN(addr + len -1))); > > > > > > > > +} > > > > > > > > + > > > > > > > > +static int unmap_memory_from_domain(struct domain *d, uint64_t > > > > > > > > addr, > > > > > > > > + uint64_t len) > > > > > > > > +{ > > > > > > > > + return iomem_deny_access(d, paddr_to_pfn(addr), > > > > > > > > + paddr_to_pfn(PAGE_ALIGN(addr + len -1))); > > > > > > > > +} > > > > > > > > > > > > > > I wonder, why we need an extra level of indirection here. And if > > > > > > > this is > > > > > > > really needed, I wonder why map(unmap)_memory* name was chosen, > > > > > > > as there is > > > > > > > no memory mapping/unmapping really happens here. > > > > > > > > > > > > > > > > > > > I've added extra indirection to hide math like > > > > > > paddr_to_pfn(PAGE_ALIGN(addr + len -1) > > > > > > so you don't have to math in each call. unmap_memory_from_domain > > > > > > called > > > > > > from 2 places, so I moved both calls to separate function. > > > > > > Although, I agree that map/unmap is not perfect name. I consider > > > > > > renaming it to mem_permit_acces and mam_deny_access. > > > > > > > > > > I haven't looked at the rest of the series. But this discussion > > > > > caught my > > > > > eye. This code implies that the address is page-aligned but the > > > > > length not. > > > > > Is that intended? > > > > > > > > > > That said, if you give permission to the domain on a full page then > > > > > it means > > > > > it may be able to access address it should not. Can you explain why > > > > > this is > > > > > fine? > > > > > > > > > > > > > The idea was that xen receives some memory from the dt_node > > > > linux,scmi_mem, > > > > then we split memory between the agents, so each agent get 1 page (we > > > > allocate 0x10 pages right now). > > > > > > Thanks for the clarification. Does this imply the guest will be able to > > > write message directly to the firmware? > > > > We used DEN0056C Specification as base. Available on: > > https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!m9pWoxBEjb8Sd1CoV5cpU8MbmLCjohYQxv2ci9tDvMmZ9oCEitqyydZ3rQWXCM5bxvIn$ > > [developer[.]arm[.]com]. > > SCMI transport is described in Section 5.1. We implemented Shared Memory > > transport. > > Firmware has N pages of the shared memory, used to communicate with Agents. > > It allocates N agents and assign a page for each agent, such as: > > ------------------------------------- > > | Agent H | Agent 1 | Agent 2 | ... | > > ------------------------------------- > > Agent H is the privilleged Hypervisor agent, which is used to do the base > > commands, > > such as getting Agent list, set\unset permissions etc. > > Hypervisor assign agent to the guest and maps page, related to the agent to > > the Guest. > > So the Guest, which is Agent 1 will get an access to Agent 1 page. > > > > Guest places SCMI message to Agent 1 memory, then sends SMC message. > > Hypervisor process SMC request, add agent id to the message parameters and > > redirects it to the Firmware. > > Based on the agent_id Firmware knows which page it should read. > > Then after permission check ( if the resetId/clockID/powerID etc from > > message > > is assigned to agent_id ) it does changes to the HW and places response to > > Agent > > shared memory and marks channel as FREE ( by setting free bit in shared > > memory ). > > Once channel is marked as free - Guest read response from the shared memory. > > So, IIUC, the hypervisor will not control what is written in the shared > memory. It will only control the SMC parameters. Is my understanding > correct? > For scmi_smc it will not. But potentially it can make some changes, or convert to the different protocol. > > > > Non-virtualized systems will work as well. OS should send SMC directly to > > the Firmware. > > > > > > > > If so, this brings a few more questions: > > > 1) What will the guest write in it? Can it contains addresses? > > Guest can write scmi request to the shared memory, which include the > > following data: > > 1) protocol_id - which protocol is requested, such as clock, power, reset > > etc > > 2) message_id - action that should be done to HW, such as do_reset or > > get_clock > > 3) message data - which includes reset_id/clock_id/power_id etc. that > > should be changed. > > Reset IDs and Clock IDs are assigned in Firmware. Guest receives ID, > > corresponding to the device from the device-tree. > > dt_node as an example: > > &avb { > > scmi_devid = <0>; > > clocks = <&scmi_clock 0>; > > power-domains = <&scmi_power 0>; > > resets = <&scmi_reset 0>; > > }; > > > > > 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. 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. Best regards, Oleksii.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |