From: Jan Beulich <jbeulich@suse.com>
To: Henry Wang <xin.wang2@amd.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
Julien Grall <julien@xen.org>,
Bertrand Marquis <bertrand.marquis@arm.com>,
Michal Orzel <michal.orzel@amd.com>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
George Dunlap <george.dunlap@citrix.com>,
xen-devel@lists.xenproject.org
Subject: Re: [PATCH v2 6/8] xen/arm: Add XEN_DOMCTL_dt_overlay DOMCTL and related operations
Date: Fri, 17 May 2024 09:11:50 +0200 [thread overview]
Message-ID: <5a979a5b-51c0-4359-813e-d9307932d8f9@suse.com> (raw)
In-Reply-To: <1f02615c-916c-4ab8-babc-17a117c56e87@amd.com>
On 17.05.2024 03:36, Henry Wang wrote:
> On 5/16/2024 8:31 PM, Jan Beulich wrote:
>> On 16.05.2024 12:03, Henry Wang wrote:
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -1190,6 +1190,17 @@ struct xen_domctl_vmtrace_op {
>>> typedef struct xen_domctl_vmtrace_op xen_domctl_vmtrace_op_t;
>>> DEFINE_XEN_GUEST_HANDLE(xen_domctl_vmtrace_op_t);
>>>
>>> +#if defined(__arm__) || defined (__aarch64__)
>> Nit: Consistent use of blanks please (also again below).
>
> Good catch. Will fix it.
>
>>> +struct xen_domctl_dt_overlay {
>>> + XEN_GUEST_HANDLE_64(const_void) overlay_fdt; /* IN: overlay fdt. */
>>> + uint32_t overlay_fdt_size; /* IN: Overlay dtb size. */
>>> +#define XEN_DOMCTL_DT_OVERLAY_ATTACH 3
>>> +#define XEN_DOMCTL_DT_OVERLAY_DETACH 4
>> While the numbers don't really matter much, picking 3 and 4 rather than,
>> say, 1 and 2 still looks a little odd.
>
> Well although I agree with you it is indeed a bit odd, the problem of
> this is that, in current implementation I reused the libxl_dt_overlay()
> (with proper backward compatible) to deliver the sysctl and domctl
> depend on the op, and we have:
> #define LIBXL_DT_OVERLAY_ADD 1
> #define LIBXL_DT_OVERLAY_REMOVE 2
> #define LIBXL_DT_OVERLAY_ATTACH 3
> #define LIBXL_DT_OVERLAY_DETACH 4
>
> Then the op-number is passed from the toolstack to Xen, and checked in
> dt_overlay_domctl(). So with this implementation the attach/detach op
> number should be 3 and 4 since 1 and 2 have different meanings.
>
> But I realized that I can also implement a similar API, say
> libxl_dt_overlay_domain() and that way we can reuse 1 and 2 and there is
> not even need to provide backward compatible of libxl_dt_overlay(). So
> would you mind sharing your preference on which approach would you like
> more? Thanks!
While I think tying together libxl and domctl values isn't a very good idea,
if you really want to do so, you'll want to add suitable checking somewhere,
alongside comments. The comments ought to keep people from changing the
values then, while the checks would need to be there for people not paying
attention.
Jan
next prev parent reply other threads:[~2024-05-17 7:12 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-16 10:03 [PATCH v2 0/8] Remaining patches for dynamic node programming using overlay dtbo Henry Wang
2024-05-16 10:03 ` [PATCH v2 1/8] xen/common/dt-overlay: Fix lock issue when add/remove the device Henry Wang
2024-05-19 10:04 ` Julien Grall
2024-05-16 10:03 ` [PATCH v2 2/8] tools/xl: Correct the help information and exit code of the dt-overlay command Henry Wang
2024-05-20 18:48 ` Jason Andryuk
2024-05-16 10:03 ` [PATCH v2 3/8] xen/arm, doc: Add a DT property to specify IOMMU for Dom0less domUs Henry Wang
2024-05-19 10:17 ` Julien Grall
2024-05-20 0:41 ` Henry Wang
2024-05-20 1:38 ` Henry Wang
2024-05-20 1:40 ` Henry Wang
2024-05-16 10:03 ` [PATCH v2 4/8] tools/arm: Introduce the "nr_spis" xl config entry Henry Wang
2024-05-20 19:13 ` Jason Andryuk
2024-05-20 23:19 ` Henry Wang
2024-05-16 10:03 ` [PATCH v2 5/8] xen/arm/gic: Allow routing/removing interrupt to running VMs Henry Wang
2024-05-17 6:03 ` Henry Wang
2024-05-19 11:08 ` Julien Grall
2024-05-20 1:01 ` Henry Wang
2024-05-20 9:50 ` Julien Grall
2024-05-21 2:29 ` Stefano Stabellini
2024-05-16 10:03 ` [PATCH v2 6/8] xen/arm: Add XEN_DOMCTL_dt_overlay DOMCTL and related operations Henry Wang
2024-05-16 12:31 ` Jan Beulich
2024-05-17 1:36 ` Henry Wang
2024-05-17 7:11 ` Jan Beulich [this message]
2024-05-16 10:03 ` [PATCH v2 7/8] tools: Introduce the "xl dt-overlay {attach,detach}" commands Henry Wang
2024-05-20 19:41 ` Jason Andryuk
2024-05-20 23:21 ` Henry Wang
2024-05-16 10:03 ` [PATCH v2 8/8] docs: Add device tree overlay documentation Henry Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5a979a5b-51c0-4359-813e-d9307932d8f9@suse.com \
--to=jbeulich@suse.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=andrew.cooper3@citrix.com \
--cc=bertrand.marquis@arm.com \
--cc=george.dunlap@citrix.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
--cc=xin.wang2@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).