public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Alexander Graf <agraf@suse.de>
To: Scott Wood <scottwood@freescale.com>
Cc: Bhushan Bharat-R65777 <Bharat.Bhushan@freescale.com>,
	"kvm-ppc@vger.kernel.org" <kvm-ppc@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest
Date: Tue, 01 Jul 2014 18:22:45 +0200	[thread overview]
Message-ID: <53B2E055.80905@suse.de> (raw)
In-Reply-To: <1404228955.2435.241.camel@snotra.buserror.net>


On 01.07.14 17:35, Scott Wood wrote:
> On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote:
>> On 01.07.14 16:58, Scott Wood wrote:
>>> On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote:
>>>> I don't think QEMU should be aware of these limitations.
>>> OK, but we should at least have some idea of how the whole thing is
>>> supposed to work, in order to determine if this is the correct behavior
>>> for QEMU.  I thought the model was that debug resources are either owned
>>> by QEMU or by the guest, and in the latter case, QEMU would never see
>>> the debug exception to begin with.
>> That's bad for a number of reasons. For starters it's different from how
>> x86 handles debug registers - and I hate to be different just for the
>> sake of being different.
> How does it work on x86?

It overwrites more-or-less random breakpoints with its own ones, but 
leaves the others intact ;).

>
>> So if we do want to declare that debug registers are owned by either
>> QEMU or the guest, we should change the semantics for all
>> architectures.
> If we want to say that ownership of the registers is shared, we need a
> plan for how that would actually work.

I think you're overengineering here :). When do people actually use 
gdbstub? Usually when they want to debug a broken guest. We can either

   * overengineer heavily and reduce the number of registers available 
to the guest to always have spares
   * overengineer a bit and turn off guest debugging completely when we 
use gdbstub
   * just leave as much alive as we can, hoping that it helps with the 
debugging

Option 3 is what x86 does - and I think it's a reasonable approach. This 
is not an interface that needs to be 100% consistent and bullet proof, 
it's a best effort to enable you to debug as much as possible.

>
>> The second problem I see is that we're disabling use cases for the sake
>> of correctness. When I use gdbstub, I usually don't want anything in the
>> way of debugging something - like if I want to debug the guest debugging
>> itself for example.
> I don't have a problem with making it possible for userspace to forcibly
> claim ownership of the debug registers -- but I don't want to advertise
> that won't mess up the guest debugging that you're trying to debug,
> without some reason to believe that.

It will mes up guest debugging, but only up to the extent that's 
necessary. We can even try to find unused breakpoint registers before we 
start killing guest ones.

>
>> So overall, I think the x86 approach is a nice middle ground between
>> usability, performance and functionality.
> You mean don't document anything other than the mechanism to get/set the
> registers and hope everything works out? :-)

There's certainly some lack of documentation here ;). Otherwise we 
probably wouldn't have this conversation :).

>
>>>>>>> one reg?
>>>>>> We are using SREGS but if required we can use one_reg.
>>>>> I thought we were preferring one reg over sregs for new functionality.
>>>> I'm personally torn on this one. The problem here is that the sregs
>>>> fields and values are already reserved. For anything we don't have an
>>>> API for yet, yes, one_reg only. IIUC we have the API here, but were
>>>> lacking the implementation.
>>>   From a QEMU perspective, are we going to want to update this at the same
>>> time as everything else, or would jumping through sregs hoops be a
>>> nuisance?  Is the long term goal to have one reg support for everything?
>> Because we need to maintain backwards compatibility, I don't think we'll
>> actually have any benefit from one reg support for everything.
> We don't need to maintain backwards compatibility with interfaces that
> were never implemented, and even where we do need to maintain
> compatibility, that doesn't mean the caller needs to use the old,
> cumbersome interface (don't assume the caller is QEMU, or that all new
> features of QEMU need to support old kernels).

Yes, that's why I'm torn for this particular case. I thought you were 
asking about the general line of thinking. In this concrete case I think 
the only thing we get from using SREGS over ONE_REG is that we don't 
have to wait for the patches to trickle into kvm-next to be able to 
apply the QEMU patches ;).

 From an esthetic point of view, I would prefer ONE_REG too.

>
>> I think we're in a path that is slow enough already to not worry about
>> performance.
> It's not just about performance, but simplicity of use, and consistency
> of API.
>
> Oh, and it looks like there already exist one reg definitions and
> implementations for most of the debug registers.

For BookE? Where?


Alex


  reply	other threads:[~2014-07-01 16:22 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-27  6:25 [PATCH 0/2] KVM: powerpc/booke: Debug interrupt injection to guest Bharat Bhushan
2014-06-27  6:25 ` [PATCH 1/2] KVM: powerpc/booke: allow debug interrupt at "debug level" Bharat Bhushan
2014-06-27  6:25 ` [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest Bharat Bhushan
2014-06-27 18:23   ` Scott Wood
2014-06-30  4:38     ` Bharat.Bhushan
2014-06-30 20:25       ` Scott Wood
2014-07-01  5:40         ` Bharat.Bhushan
2014-07-01  6:23         ` Alexander Graf
2014-07-01 10:06           ` Bharat.Bhushan
2014-07-01 10:12             ` Alexander Graf
2014-07-01 10:30               ` Bharat.Bhushan
2014-07-01 10:48                 ` Alexander Graf
2014-07-01 14:58           ` Scott Wood
2014-07-01 15:04             ` Alexander Graf
2014-07-01 15:35               ` Scott Wood
2014-07-01 16:22                 ` Alexander Graf [this message]
2014-07-01 16:40                   ` Scott Wood
2014-07-02 11:37                     ` Bharat.Bhushan
2014-07-02 17:28                     ` Bharat.Bhushan
2014-07-03 11:36                       ` Alexander Graf

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=53B2E055.80905@suse.de \
    --to=agraf@suse.de \
    --cc=Bharat.Bhushan@freescale.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=scottwood@freescale.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