qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: <peng.hao2@zte.com.cn>
To: peter.maydell@linaro.org
Cc: drjones@redhat.com, qemu-arm@nongnu.org, philmd@redhat.com,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH V10 7/9] hw/misc/pvpanic: preparing foradding configure interface
Date: Sat, 1 Dec 2018 17:28:49 +0800 (CST)	[thread overview]
Message-ID: <201812011728499706543@zte.com.cn> (raw)
In-Reply-To: <CAFEAcA9Od4vjo2EeEXhP1iE_XONaK8H22L9eq1qw3w6C2nR_cQ@mail.gmail.com>

>On Fri, 30 Nov 2018 at 16:14, Andrew Jones <drjones@redhat.com> wrote:
>>
>> On Fri, Nov 30, 2018 at 03:57:13PM +0000, Peter Maydell wrote:
>> > On Fri, 30 Nov 2018 at 15:56, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > > I suspect the reason you've done this is that you're
>> > > trying to get "-device pvpanic" to work on the command
>> > > line. I recommend not trying to get that to work.
>> > > MMIO devices are not pluggable devices, and -device
>> > > is not expected to work with them.
>> >
>> > If you do want a pluggable pvpanic device for the virt
>> > board then you should implement it as a PCI device,
>> > incidentally.
>> >
>>
>> We'd have to allocate it a PCI device ID, but I guess that's OK as
>> there are plenty of IDs left for 1b36. I'm not sure it's worth it
>> though. Phil asked that this device by user creatable, but maybe
>> that's not necessary. Maybe we just need a mach-virt compat boolean
>> and then to always provide this device for 4.0 and later machines?
>
>Yes, if it's just an mmio device then we should either:
>* default to providing it, with the usual flag to say "don't create
>in older virt board versions", and also a machine property to
>disable it manually (like we do with the ITS)
>* default to not providing it at all, and have a machine
>property to enable it (like we do with the IOMMU)
>Which is all doable, but every time we do that it makes the
>virt board code that extra bit more complicated (we have
>half a dozen machine properties on it already).
>
>This kind of thing is why a PCI device is cleaner -- it just
>works on any machine with a PCI controller, it by default is
>something that the user can provide if they want to and not if
>they don't, and it's not a custom UI that management layers
>might need to special-case. The guest does need to do a bit
>of PCI probing and setup initially, but then it can just leave
>the MMIO BAR permanently mapped and write to that address
>when the guest panics.
>
Usually, it doesn't matter if there is this pvpanic device or not, so I 
haven't considered configurability from the beginning.
 I didn't expect you to strongly recommend the use of PCI devices. 
Doing so is not just going to roll back some of the code in the kernel... 
I'll think about your suggestions.
thanks.
>thanks
-- PMM

  reply	other threads:[~2018-12-01  9:28 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-28 12:12 [Qemu-devel] [PATCH V10 0/9] add pvpanic mmio support Peng Hao
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 1/9] hw/misc/pvpanic: Build the pvpanic device in $(common-obj) Peng Hao
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 2/9] hw/misc/pvpanic: Cosmetic renaming Peng Hao
2018-11-30 15:39   ` Peter Maydell
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 3/9] hw/misc/pvpanic: Add the MMIO interface Peng Hao
2018-11-30 15:41   ` Peter Maydell
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 4/9] hw/arm/virt: Use the pvpanic device Peng Hao
2018-11-30 15:48   ` Peter Maydell
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 5/9] hw/arm/virt: add pvpanic device in virt acpi table Peng Hao
2018-11-30 15:51   ` Peter Maydell
2018-11-30 16:06   ` Andrew Jones
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 6/9] hw/misc/pvpanic: add configure query interface Peng Hao
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 7/9] hw/misc/pvpanic: preparing for adding configure interface Peng Hao
2018-11-30 15:56   ` Peter Maydell
2018-11-30 15:57     ` Peter Maydell
2018-11-30 16:14       ` Andrew Jones
2018-11-30 16:21         ` Peter Maydell
2018-12-01  9:28           ` peng.hao2 [this message]
2018-12-01 12:05             ` [Qemu-devel] [PATCH V10 7/9] hw/misc/pvpanic: preparing foradding " Peter Maydell
2018-12-01  9:11     ` [Qemu-devel] [PATCH V10 7/9] hw/misc/pvpanic: preparing for adding " peng.hao2
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 8/9] hw/misc/pvpanic: realize the " Peng Hao
2018-11-30 15:53   ` Peter Maydell
2018-11-28 12:12 ` [Qemu-devel] [PATCH V10 9/9] pvpanic : update pvpanic document Peng Hao
2018-11-30 16:00   ` Andrew Jones

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=201812011728499706543@zte.com.cn \
    --to=peng.hao2@zte.com.cn \
    --cc=drjones@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.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 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 NNTP newsgroup(s).