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

Re: [PATCH] xen/arm: Move static event channel feature to a separate module


  • To: Julien Grall <julien@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Michal Orzel <michal.orzel@xxxxxxx>
  • Date: Wed, 29 Nov 2023 18:41:40 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=xen.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • 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=PQP+IWU0LRPAnxHMIF6ux3j/NrHaaJ1QsdLr/6q8EtA=; b=W0mCdtiLRsNhovuJqzdnaKKkhF6Svq1+8/hYy8rp7MMeD3ROG9AM6EfbmwHY1KoP0PfFv5HWCN0P3PLQBthYHHDAJI7S8ty41id1uba0hiOhwp6K+uDNY1RT2rOZw7fCwiH3gREA9eQXT2XeTgpQjkro26Y5Eh8+lJ8j0Aj7ma6WY0XiVJOlYl9nOc3kLnR8xZ41kUrjnki3vw/jBpO1mK1dbRNW/8bWxqc9BiRH5XYmthxmMoBU5h4XrEV4NmrlbUF9rZIPemzV0lnsyN7XOp1gn7dD5Qs7xtwgHjojd4jE2+fvJHPRtONk8PInJwTuY3N4K0Q9vFmlQshnR11J4A==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DliP1s5SMekwAonz5/SWDvOr/RsO3fDcoC8eDfXIbq4i2mE8BDK6b+lY0TO4svNv7pDJvfrFn9pMFmvwsHLLFOQrZIblB9P2Y3PmoqfNAY9IPdhSoYvduudRFxJgSrWNMzPnBF4AUONvPvZu8qvQ6pvqgpKsQObAIQsclR7taRZ74GzTcAjsy970QEX+LCkNh9N/cEWSSh7PmGwO5EA3pOmcrZBBATUVKNNkrcZluYqkxYgTE6uaym86efDF2aGN7ocSzbzfR02h8XHHjZgfrwOeolK5iwqnTGqOY+3f/EwKbBPpyWP5cmhSRpV4FNNzSrBawzVmVvVua+u/egVtkQ==
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Wed, 29 Nov 2023 17:41:56 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Julien,

On 29/11/2023 18:17, Julien Grall wrote:
> 
> 
> Hi Michal
> 
> On 29/11/2023 16:34, Michal Orzel wrote:
>> Move static event channel feature related code to a separate module
>> (static-evtchn.{c,h}) in the spirit of fine granular configuration, so
>> that the feature can be disabled if not needed.
>>
>> Introduce Kconfig option CONFIG_STATIC_EVTCHN, enabled by default (to
>> keep the current behavior) dependent on CONFIG_DOM0LESS. While it could
>> be possible to create a loopback connection for dom0 only, this use case
>> does not really need this feature and all the docs and commit messages
>> refer explicitly to the use in dom0less system.
>>
>> The only function visible externally is alloc_static_evtchn(), so move
>> the prototype to static-evtchn.h and provide a stub in case a feature
>> is disabled. Guard static_evtchn_created in struct dt_device_node and
>> move its helpers to static-evtchn.h.
> 
> I guess this is a matter of taste, but I think
> dt_device_set_static_evtchn_created() and
> dt_device_static_evtchn_created() are better suited in device_tree.h
> because they are touching a field in the device tree structure.
> 
> This would stay consistent with the helper dt_device_set_protected()
> which is only used by the IOMMU code yet is defined in device_tree.h.
Good point about consistency. I will keep the helpers in device_tree.h + add 
guards.

> 
> That said, I could settle on defining the two helpers in the *.c
> directly because they are not meant to be used outside of a single *.c.
> 
> Simarly...
> 
>> diff --git a/xen/arch/arm/include/asm/static-evtchn.h 
>> b/xen/arch/arm/include/asm/static-evtchn.h
>> new file mode 100644
>> index 000000000000..472673fae345
>> --- /dev/null
>> +++ b/xen/arch/arm/include/asm/static-evtchn.h
>> @@ -0,0 +1,41 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +
>> +#ifndef __ASM_STATIC_EVTCHN_H_
>> +#define __ASM_STATIC_EVTCHN_H_
>> +
>> +#ifdef CONFIG_STATIC_EVTCHN
>> +
>> +#include <xen/device_tree.h>
>> +
>> +#define STATIC_EVTCHN_NODE_SIZE_CELLS 2
> 
> ... this used to be defined in domain_build.c. AFAICT the only use is
> now in static-evtchn.c. So why is it defined in the header?
> 
> If this is moved in the *.c, then the header static-evtchn.h would only
> contain alloc_static_evtchn(). This could be moved in domain_build.h or
> setup.h (I don't have any preference).
Apart from a prototype, we still need a stub. Therefore I would prefer to still 
have a header (will
be needed for future upgrades e.g. port exposure in fdt) and move the prototype 
and a stub there (the macro
I can move to *.c). It just looks better for me + we reduce ifdefery in 
domain_build/setup.h.
Would you be ok with that?

~Michal




 


Rackspace

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