Re: [PATCH v6 04/11] common: add vmtrace_pt_size domain parameter

From: Jan Beulich <jbeulich@suse.com>
To: "Michał Leszczyński" <michal.leszczynski@cert.pl>
Cc: "Julien Grall" <julien@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	tamas.lengyel@intel.com, "Wei Liu" <wl@xen.org>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	luwei.kang@intel.com, xen-devel@lists.xenproject.org,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH v6 04/11] common: add vmtrace_pt_size domain parameter
Date: Fri, 7 Aug 2020 16:29:27 +0200	[thread overview]
Message-ID: <93de2427-d5ef-32b0-d5b0-dd9759c884f9@suse.com> (raw)
In-Reply-To: <036bc768bfb074269d9bd4530304a11170b7142d.1594150543.git.michal.leszczynski@cert.pl>

On 07.07.2020 21:39, Michał Leszczyński wrote:
> @@ -443,6 +449,9 @@ struct domain *domain_create(domid_t domid,
>          d->nr_pirqs = min(d->nr_pirqs, nr_irqs);
>  
>          radix_tree_init(&d->pirq_tree);
> +
> +        if ( config->processor_trace_buf_kb )
> +            d->processor_trace_buf_kb = config->processor_trace_buf_kb;

There's no real need for the if, is there?

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -92,6 +92,7 @@ struct xen_domctl_createdomain {
>      uint32_t max_evtchn_port;
>      int32_t max_grant_frames;
>      int32_t max_maptrack_frames;
> +    uint32_t processor_trace_buf_kb;

Adding a new field to an existing interface struct requires bumping
of the interface version, unless that has already happened during a
release cycle.

Here I agree with the choice of uint32_t, but ...

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -457,6 +457,9 @@ struct domain
>      unsigned    pbuf_idx;
>      spinlock_t  pbuf_lock;
>  
> +    /* Used by vmtrace features */
> +    uint32_t    processor_trace_buf_kb;

... why a fixed size type here? unsigned int will do fine, won't it?

Jan