Re: [Xen-devel] [PATCH v2 4/5] pci: Add INTERFACE_CONVENTIONAL_PCI_DEVICE to Conventional PCI devices

2017-09-27 Thread David Gibson
On Wed, Sep 27, 2017 at 04:56:34PM -0300, Eduardo Habkost wrote:
> Add INTERFACE_CONVENTIONAL_PCI_DEVICE to all direct subtypes of
> TYPE_PCI_DEVICE, except:
> 
> 1) The ones that already have INTERFACE_PCIE_DEVICE set:
> 
> * base-xhci
> * e1000e
> * nvme
> * pvscsi
> * vfio-pci
> * virtio-pci
> * vmxnet3
> 
> 2) base-pci-bridge
> 
> Not all PCI bridges are Conventional PCI devices, so
> INTERFACE_CONVENTIONAL_PCI_DEVICE is added only to the subtypes
> that are actually Conventional PCI:
> 
> * dec-21154-p2p-bridge
> * i82801b11-bridge
> * pbm-bridge
> * pci-bridge
> 
> The direct subtypes of base-pci-bridge not touched by this patch
> are:
> 
> * xilinx-pcie-root: Already marked as PCIe-only.
> * pcie-pci-bridge: Already marked as PCIe-only.
> * pcie-port: all non-abstract subtypes of pcie-port are already
>   marked as PCIe-only devices.
> 
> 3) megasas-base
> 
> Not all megasas devices are Conventional PCI devices, so the
> interface names are added to the subclasses registered by
> megasas_register_types(), according to information in the
> megasas_devices[] array.
> 
> "megasas-gen2" already implements INTERFACE_PCIE_DEVICE, so add
> INTERFACE_CONVENTIONAL_PCI_DEVICE only to "megasas".
> 
> Acked-by: Alberto Garcia 
> Acked-by: John Snow 
> Acked-by: Anthony PERARD 
> Signed-off-by: Eduardo Habkost 

Reviewed-by: David Gibson 

and for the ppc devices

Acked-by: David Gibson 

> ---
> Changes v1 -> v2:
> * s/legacy/conventional/
>   * Suggested-by: Alex Williamson 
> * Note about pcie-pci-bridge on commit message.
> * New devices: sungem, sunhme
> 
> Cc: "Michael S. Tsirkin" 
> Cc: Igor Mammedov 
> Cc: Gerd Hoffmann 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Stefano Stabellini 
> Cc: Anthony Perard 
> Cc: John Snow 
> Cc: Alberto Garcia 
> Cc: Aurelien Jarno 
> Cc: Yongbok Kim 
> Cc: Jiri Slaby 
> Cc: Alexander Graf 
> Cc: Marcel Apfelbaum 
> Cc: Jason Wang 
> Cc: Jiri Pirko 
> Cc: "Hervé Poussineau" 
> Cc: Peter Maydell 
> Cc: David Gibson 
> Cc: Hannes Reinecke 
> Cc: Mark Cave-Ayland 
> Cc: Artyom Tarasenko 
> Cc: Alex Williamson 
> Cc: qemu-de...@nongnu.org
> Cc: xen-de...@lists.xenproject.org
> Cc: qemu-bl...@nongnu.org
> Cc: qemu-...@nongnu.org
> Cc: qemu-...@nongnu.org
> ---
>  hw/acpi/piix4.c |  1 +
>  hw/audio/ac97.c |  4 
>  hw/audio/es1370.c   |  4 
>  hw/audio/intel-hda.c|  4 
>  hw/char/serial-pci.c| 12 
>  hw/display/cirrus_vga.c |  4 
>  hw/display/qxl.c|  4 
>  hw/display/sm501.c  |  4 
>  hw/display/vga-pci.c|  4 
>  hw/display/vmware_vga.c |  4 
>  hw/i2c/smbus_ich9.c |  4 
>  hw/i386/amd_iommu.c |  4 
>  hw/i386/kvm/pci-assign.c|  4 
>  hw/i386/pc_piix.c   |  4 
>  hw/i386/xen/xen_platform.c  |  4 
>  hw/i386/xen/xen_pvdevice.c  |  4 
>  hw/ide/ich.c|  4 
>  hw/ide/pci.c|  4 
>  hw/ipack/tpci200.c  |  4 
>  hw/isa/i82378.c |  4 
>  hw/isa/lpc_ich9.c   |  1 +
>  hw/isa/piix4.c  |  4 
>  hw/isa/vt82c686.c   | 16 
>  hw/mips/gt64xxx_pci.c   |  4 
>  hw/misc/edu.c   |  5 +
>  hw/misc/ivshmem.c   |  4 
>  hw/misc/macio/macio.c   |  4 
>  hw/misc/pci-testdev.c   |  4 
>  hw/net/e1000.c  |  4 
>  hw/net/eepro100.c   |  4 
>  hw/net/ne2000.c |  4 
>  hw/net/pcnet-pci.c  |  4 
>  hw/net/rocker/rocker.c  |  4 
>  hw/net/rtl8139.c|  4 
>  hw/net/sungem.c |  4 
>  hw/net/sunhme.c |  4 
>  hw/pci-bridge/dec.c |  8 
>  hw/pci-bridge/i82801b11.c   |  4 
>  hw/pci-bridge/pci_bridge_dev.c  |  1 +
>  hw/pci-bridge/pci_expander_bridge.c |  8 
>  hw/pci-host/apb.c   |  8 
>  hw/pci-host/bonito.c|  4 
>  hw/pci-host/gpex.c  |  4 
>  hw/pci-host/grackle.c   |  4 
>  hw/pci-host/piix.c  |  8 
>  hw/pci-host/ppce500.c   |  4 
>  hw/pci-host

Re: [Xen-devel] [PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET

2017-05-10 Thread David Gibson
On Mon, May 08, 2017 at 09:32:42AM -0500, Eric Blake wrote:
> On 05/08/2017 12:26 AM, David Gibson wrote:
> > On Fri, May 05, 2017 at 02:38:08PM -0500, Eric Blake wrote:
> >> Time to wire up all the call sites that request a shutdown or
> >> reset to use the enum added in the previous patch.
> >>
> >> It would have been less churn to keep the common case with no
> >> arguments as meaning guest-triggered, and only modified the
> >> host-triggered code paths, via a wrapper function, but then we'd
> >> still have to audit that I didn't miss any host-triggered spots;
> >> changing the signature forces us to double-check that I correctly
> >> categorized all callers.
> >>
> >> Since command line options can change whether a guest reset request
> >> causes an actual reset vs. a shutdown, it's easy to also add the
> >> information to reset requests.
> >>
> >> Replay adds a FIXME to preserve the cause across the replay stream,
> >> that will be tackled in the next patch.
> >>
> >> Signed-off-by: Eric Blake 
> >> Acked-by: David Gibson  [ppc parts]
> >> Reviewed-by: Mark Cave-Ayland  [SPARC part]
> > 
> > [snip]
> > 
> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >> index 9f18f75..2735fe9 100644
> >> --- a/hw/ppc/spapr_hcall.c
> >> +++ b/hw/ppc/spapr_hcall.c
> >> @@ -1166,7 +1166,7 @@ static target_ulong 
> >> h_client_architecture_support(PowerPCCPU *cpu,
> >>  spapr_ovec_cleanup(ov5_updates);
> >>
> >>  if (spapr->cas_reboot) {
> >> -qemu_system_reset_request();
> >> +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > 
> > I'm not 100% sure about this one, since I'm not sure 100% of how the
> > different enum values are defined.  This one is tripped when feature
> > negotiation between firmware and guest can't be satisfied without
> > rebooting (next time round the firmware will use some different
> > options).
> 
> Patch 2/5 introduced the enum.  The biggest part of the patch (for now)
> is that anything SHUTDOWN_CAUSE_HOST_ will be exposed to the QMP client
> as host-triggered, anything SHUTDOWN_CAUSE_GUEST_ will be exposed as
> guest-triggered.  I basically used SHUTDOWN_CAUSE_GUEST_RESET for any
> call to qemu_system_reset_requst() underneath the hw/ tree, because the
> hw/ tree is emulating guest behavior and therefore it is presumably a
> reset caused by a guest request.

Ok.

> > So it's essentially a firmware/hypervisor triggered reset, but one
> > that should only ever be tripped during early guest boot.  Is
> > CAUSE_GUEST_RESET correct for that?
> 
> Of course, I'm not an export on SPAPR, so I'll happily change it to
> anything else if you think that is more appropriate. But the rule of
> thumb I went by is whether this is qemu emulating a bare-metal
> reset/shutdown, vs. qemu killing the guest without waiting for guest
> instructions to reach some magic
> memory/register/ACPI/who-knows-what-else request.  While it may happen
> only early during guest boot, it is still the guest firmware that is
> requesting it, and not qemu causing a unilateral death.

So, I think GUEST_RESET is the right choice here.  The distinctions
are blurry, because PAPR is built as a paravirtualized platform -
there is no bare metal equivalent.  For example, on PowerVM this
really would be initiated by guest firmware, but it's working in
communication with the hypervisor.  But with qemu and KVM, we actually
implement all this negotiation logic in qemu directly (this is easier
than having complex communication channels between guest firware and
qemu).

But I guess the point is that this is a "business as usual" reboot and
the guest is expected to continue booting at some point after this,
rather than being killed by qemu.

Things get more complicated still if we think about what happens if
this feature negotiation fails - then we can't boot the guest OS.  At
the moment the few cases where this happens, I think we just exit
qemu, but notifying this as a host caused shutdown might be
appropriate.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET

2017-05-07 Thread David Gibson
On Fri, May 05, 2017 at 02:38:08PM -0500, Eric Blake wrote:
> Time to wire up all the call sites that request a shutdown or
> reset to use the enum added in the previous patch.
> 
> It would have been less churn to keep the common case with no
> arguments as meaning guest-triggered, and only modified the
> host-triggered code paths, via a wrapper function, but then we'd
> still have to audit that I didn't miss any host-triggered spots;
> changing the signature forces us to double-check that I correctly
> categorized all callers.
> 
> Since command line options can change whether a guest reset request
> causes an actual reset vs. a shutdown, it's easy to also add the
> information to reset requests.
> 
> Replay adds a FIXME to preserve the cause across the replay stream,
> that will be tackled in the next patch.
> 
> Signed-off-by: Eric Blake 
> Acked-by: David Gibson  [ppc parts]
> Reviewed-by: Mark Cave-Ayland  [SPARC part]

[snip]

> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 9f18f75..2735fe9 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1166,7 +1166,7 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>  spapr_ovec_cleanup(ov5_updates);
> 
>  if (spapr->cas_reboot) {
> -qemu_system_reset_request();
> +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);

I'm not 100% sure about this one, since I'm not sure 100% of how the
different enum values are defined.  This one is tripped when feature
negotiation between firmware and guest can't be satisfied without
rebooting (next time round the firmware will use some different
options).

So it's essentially a firmware/hypervisor triggered reset, but one
that should only ever be tripped during early guest boot.  Is
CAUSE_GUEST_RESET correct for that?

Apart from this, ppc changes

Acked-by: David Gibson 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v5 3/4] shutdown: Add source information to SHUTDOWN and RESET

2017-05-01 Thread David Gibson
On Thu, Apr 27, 2017 at 09:13:16PM -0500, Eric Blake wrote:
> Libvirt would like to be able to distinguish between a SHUTDOWN
> event triggered solely by guest request and one triggered by a
> SIGTERM or other action on the host.  While qemu_kill_report() is
> already able to tell whether a shutdown was triggered by a host
> signal (but NOT by a host UI event, such as clicking the X on
> the window), that information was then lost after being printed
> to stderr.  The previous patch prepped things to use an enum
> internally; now it's time to wire it up through all callers, and
> to extend the SHUTDOWN and RESET events to report the details.
> 
> Enhance the shutdown request path to take a parameter of which
> way it is being triggered, and update ALL callers.  It would have
> been less churn to keep the common case with no arguments as
> meaning guest-triggered, and only modified the host-triggered
> code paths, via a wrapper function, but then we'd still have to
> audit that I didn't miss any host-triggered spots; changing the
> signature forces us to double-check that I correctly categorized
> all callers.
> 
> Since command line options can change whether a guest reset request
> causes an actual reset vs. a shutdown, it's easy to also add the
> information to the RESET event, even though libvirt has not yet
> expressed a need to know that.
> 
> For the moment, we keep the enum ShutdownCause for internal use
> only, and merely expose a single boolean of 'guest':true|false
> to the QMP client; this is because we don't yet have evidence that
> the further distinctions will be useful, or whether the addition
> of new enum members would cause problems to clients coded to an
> older version of the enum.
> 
> Update expected iotest outputs to match the new data.
> 
> Here is output from 'virsh qemu-monitor-event --loop' with the
> patch installed:
> 
> event SHUTDOWN at 1492639680.731251 for domain fedora_13: {"guest":true}
> event STOP at 1492639680.732116 for domain fedora_13: 
> event SHUTDOWN at 1492639680.732830 for domain fedora_13: {"guest":false}
> 
> Note that libvirt runs qemu with -no-quit: the first SHUTDOWN event
> was triggered by an action I took directly in the guest (shutdown -h),
> at which point qemu stops the vcpus and waits for libvirt to do any
> final cleanups; the second SHUTDOWN event is the result of libvirt
> sending SIGTERM now that it has completed cleanup.
> 
> The replay driver needs a followup patch if we want to be able to
> faithfully replay the difference between a host- and guest-initiated
> shutdown (for now, the replayed event is always attributed to host).
> 
> See also https://bugzilla.redhat.com/1384007
> 
> Signed-off-by: Eric Blake 

ppc parts

Acked-by: David Gibson 

> 
> ---
> v5: drop accidental addition of unrelated files
> v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
> v3: retitle again, fix qemu-iotests, use enum rather than raw bool
> in all callers
> v2: retitle (was "event: Add signal information to SHUTDOWN"),
> completely rework to post bool based on whether it is guest-initiated
> v1: initial submission, exposing just Unix signals from host
> ---
>  qapi/event.json | 17 +
>  include/sysemu/sysemu.h |  4 ++--
>  vl.c| 22 +++---
>  hw/acpi/core.c  |  4 ++--
>  hw/arm/highbank.c   |  4 ++--
>  hw/arm/integratorcp.c   |  2 +-
>  hw/arm/musicpal.c   |  2 +-
>  hw/arm/omap1.c  | 10 ++
>  hw/arm/omap2.c  |  2 +-
>  hw/arm/spitz.c  |  2 +-
>  hw/arm/stellaris.c  |  2 +-
>  hw/arm/tosa.c   |  2 +-
>  hw/i386/pc.c|  2 +-
>  hw/i386/xen/xen-hvm.c   |  2 +-
>  hw/input/pckbd.c|  4 ++--
>  hw/ipmi/ipmi.c  |  4 ++--
>  hw/isa/lpc_ich9.c   |  2 +-
>  hw/mips/boston.c|  2 +-
>  hw/mips/mips_malta.c|  2 +-
>  hw/mips/mips_r4k.c  |  4 ++--
>  hw/misc/arm_sysctl.c|  8 
>  hw/misc/cbus.c  |  2 +-
>  hw/misc/macio/cuda.c|  4 ++--
>  hw/misc/slavio_misc.c   |  4 ++--
>  hw/misc/zynq_slcr.c |  2 +-
>  hw/pci-host/apb.c   |  4 ++--
>  hw/pci-host/bonito.c|  2 +-
>  hw/pci-host/piix.c  |  2 +-
>  hw/ppc/e500.c   |  2 +-
>  hw/ppc/mpc8544_guts.c   |  2 +-
>  hw/ppc/ppc.c|  2 +-
>  hw/ppc/ppc405_uc.c  |  2 +-
>  hw/ppc/spapr_hcall.c|  2 +-
>  hw/ppc/spapr_rtas.c |  4 ++--
>  hw/s390x/ipl.c  |  2 +-
>