All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michał Leszczyński" <michal.leszczynski@cert.pl>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Kevin Tian" <kevin.tian@intel.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Jun Nakajima" <jun.nakajima@intel.com>, "Wei Liu" <wl@xen.org>,
	"Tamas K Lengyel" <tamas.lengyel@intel.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Kang, Luwei" <luwei.kang@intel.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	xen-devel@lists.xenproject.org,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH v3 4/7] x86/vmx: add do_vmtrace_op
Date: Mon, 29 Jun 2020 11:52:38 +0200 (CEST)	[thread overview]
Message-ID: <1499783228.15242029.1593424358779.JavaMail.zimbra@cert.pl> (raw)
In-Reply-To: <250cd39e-aa51-5397-93f9-b863e4f51269@citrix.com>

----- 23 cze 2020 o 13:54, Andrew Cooper andrew.cooper3@citrix.com napisał(a):
> Overall, the moving parts of this series needs to split out into rather
> more patches.
> 
> First, in patch 3, the hvm_funcs.pt_supported isn't the place for that
> to live.  You want a global "bool vmtrace_supported" in common/domain.c
> which vmx_init_vmcs_config() sets, and the ARM code can set in the
> future when CoreSight is added.
> 
> Next, you want a patch in isolation which adds vmtrace_pt_size (or
> whatever it ends up being) to createdomain, where all
> allocation/deallocation logic lives in common/domain.c.  The spinlock
> (if its needed, but I don't think it is) wants initialising early in
> domain_create(), alongside d->pbuf_lock, and you also need an extra
> clause in sanitise_domain_config() which rejects a vmtrace setting if
> vmtrace isn't supported.  You'll need to put the struct page_info *
> pointer to the memory allocation in struct vcpu, and adjust the vcpu
> create/destroy logic appropriately.
> 
> Next, you want a patch doing the acquire resource logic for userspace to
> map the buffers.
> 
> Next, you want a patch to introduce a domctl with the various runtime
> enable/disable settings which were in an hvmop here.
> 
> Next, you want a patch to do the VMX plumbing, both at create, and runtime.
> 
> This ought to lay the logic out in a way which is extendable to x86 PV
> guests and ARM CoreSight, and oughtn't to explode when creating guests
> on non-Intel hardware.
> 
> Thanks,
> 
> ~Andrew


Thanks for your review, I'm almost done addressing all these remarks.
I've converted HVMOP to DOMCTL and splitted patches to smaller pieces.

I will send v4 soon.


Best regards,
Michał Leszczyński


  reply	other threads:[~2020-06-29  9:53 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-22 18:06 [PATCH v3 0/7] Implement support for external IPT monitoring Michał Leszczyński
2020-06-22 18:10 ` [PATCH v3 1/7] memory: batch processing in acquire_resource() Michał Leszczyński
2020-06-22 18:10 ` [PATCH v3 2/7] x86/vmx: add Intel PT MSR definitions Michał Leszczyński
2020-06-22 18:11 ` [PATCH v3 3/7] x86/vmx: add IPT cpu feature Michał Leszczyński
2020-06-22 18:11 ` [PATCH v3 4/7] x86/vmx: add do_vmtrace_op Michał Leszczyński
2020-06-23 11:54   ` Andrew Cooper
2020-06-29  9:52     ` Michał Leszczyński [this message]
2020-06-22 18:12 ` [PATCH v3 5/7] tools/libxc: add xc_vmtrace_* functions Michał Leszczyński
2020-06-26 11:50   ` Wei Liu
2020-06-22 18:12 ` [PATCH v3 6/7] tools/libxl: add vmtrace_pt_size parameter Michał Leszczyński
2020-06-26 11:52   ` Wei Liu
2020-06-22 18:12 ` [PATCH v3 7/7] tools/proctrace: add proctrace tool Michał Leszczyński
2020-06-23  9:35   ` Tamas K Lengyel
2020-06-26 11:48   ` Wei Liu
2020-06-26 13:24     ` Ian Jackson
2020-06-29 15:27       ` Tamas K Lengyel
2020-07-01 11:01         ` Ian Jackson
2020-06-22 18:19 ` [PATCH v3 0/7] Implement support for external IPT monitoring Michał Leszczyński

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=1499783228.15242029.1593424358779.JavaMail.zimbra@cert.pl \
    --to=michal.leszczynski@cert.pl \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=jun.nakajima@intel.com \
    --cc=kevin.tian@intel.com \
    --cc=luwei.kang@intel.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tamas.lengyel@intel.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.