1. 15 5月, 2020 9 次提交
    • M
      Drop more @errp parameters after previous commit · 40c2281c
      Markus Armbruster 提交于
      Several functions can't fail anymore: ich9_pm_add_properties(),
      device_add_bootindex_property(), ppc_compat_add_property(),
      spapr_caps_add_properties(), PropertyInfo.create().  Drop their @errp
      parameter.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-16-armbru@redhat.com>
      40c2281c
    • M
      qom: Drop parameter @errp of object_property_add() & friends · d2623129
      Markus Armbruster 提交于
      The only way object_property_add() can fail is when a property with
      the same name already exists.  Since our property names are all
      hardcoded, failure is a programming error, and the appropriate way to
      handle it is passing &error_abort.
      
      Same for its variants, except for object_property_add_child(), which
      additionally fails when the child already has a parent.  Parentage is
      also under program control, so this is a programming error, too.
      
      We have a bit over 500 callers.  Almost half of them pass
      &error_abort, slightly fewer ignore errors, one test case handles
      errors, and the remaining few callers pass them to their own callers.
      
      The previous few commits demonstrated once again that ignoring
      programming errors is a bad idea.
      
      Of the few ones that pass on errors, several violate the Error API.
      The Error ** argument must be NULL, &error_abort, &error_fatal, or a
      pointer to a variable containing NULL.  Passing an argument of the
      latter kind twice without clearing it in between is wrong: if the
      first call sets an error, it no longer points to NULL for the second
      call.  ich9_pm_add_properties(), sparc32_ledma_realize(),
      sparc32_dma_realize(), xilinx_axidma_realize(), xilinx_enet_realize()
      are wrong that way.
      
      When the one appropriate choice of argument is &error_abort, letting
      users pick the argument is a bad idea.
      
      Drop parameter @errp and assert the preconditions instead.
      
      There's one exception to "duplicate property name is a programming
      error": the way object_property_add() implements the magic (and
      undocumented) "automatic arrayification".  Don't drop @errp there.
      Instead, rename object_property_add() to object_property_try_add(),
      and add the obvious wrapper object_property_add().
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-15-armbru@redhat.com>
      [Two semantic rebase conflicts resolved]
      d2623129
    • M
      qdev: Clean up qdev_connect_gpio_out_named() · 9f742c28
      Markus Armbruster 提交于
      Both qdev_connect_gpio_out_named() and device_set_realized() put
      objects without a parent into the "/machine/unattached/" orphanage.
      
      qdev_connect_gpio_out_named() needs a lengthy comment to explain how
      it works.  It exploits that object_property_add_child() can fail only
      when we got a parent already, and ignoring that error does what we
      want.  True.  If it failed due to "duplicate property", we'd be in
      trouble, but that would be a programming error.
      
      device_set_realized() is cleaner: it checks whether we need a parent,
      then calls object_property_add_child(), aborting on failure.  No need
      for a comment, and programming errors get caught.
      
      Change qdev_connect_gpio_out_named() to match.
      
      Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Message-Id: <20200505152926.18877-14-armbru@redhat.com>
      9f742c28
    • M
      hw/arm/bcm2835: Drop futile attempts at QOM-adopting memory · 5462cc81
      Markus Armbruster 提交于
      The "bcm2835-peripherals" device's .instance_init() method
      bcm2835_peripherals_init() attempts to make two memory regions QOM
      children of the device.  This is futile, because memory_region_init()
      already did.  The errors are ignored (a later commit will change
      that).  Drop the useless calls.
      
      Cc: Peter Maydell <peter.maydell@linaro.org>
      Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
      Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>
      Cc: qemu-arm@nongnu.org
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Message-Id: <20200505152926.18877-13-armbru@redhat.com>
      5462cc81
    • M
      e1000: Don't run e1000_instance_init() twice · a13f2042
      Markus Armbruster 提交于
      QOM object initialization runs .instance_init() for the type and all
      its supertypes; see object_init_with_type().
      
      Both TYPE_E1000_BASE and its concrete subtypes set .instance_init() to
      e1000_instance_init().  For the concrete subtypes, it duly gets run
      twice.  The second run fails, but the error gets ignored (a later
      commit will change that).
      
      Remove it from the subtypes.
      
      Cc: Jason Wang <jasowang@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Acked-by: NJason Wang <jasowang@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-12-armbru@redhat.com>
      a13f2042
    • M
      hw/isa/superio: Make the components QOM children · e5084306
      Markus Armbruster 提交于
      isa_superio_realize() attempts to make isa-parallel and isa-serial QOM
      children, but this does not work, because it calls
      object_property_add_child() after realizing with qdev_init_nofail().
      Realizing a device without a parent gives it one: it gets put into the
      "/machine/unattached/" orphanage.  The extra
      object_property_add_child() fails, and isa_superio_realize() ignores
      the error.
      
      Move the object_property_add_child() before qdev_init_nofail(), and
      pass &error_abort.
      
      For the other components, isa_superio_realize() doesn't even try.  Add
      object_property_add_child() there.
      
      This affects machines 40p, clipper and fulong2e.
      
      For instance, fulong2e has its vt82c686b-superio (which is an
      isa-superio) at /machine/unattached/device[9].  Before the patch, its
      components are at /machine/unattached/device[10] .. [14].  Afterwards,
      they are at
      /machine/unattached/device[9]/{parallel0,serial0,serial1,isa-fdc,i8042}.
      
      Cc: Michael S. Tsirkin <mst@redhat.com>
      Cc: Paolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Tested-by: NPhilippe Mathieu-Daudé <philmd@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-11-armbru@redhat.com>
      e5084306
    • M
      qom: Drop object_property_set_description() parameter @errp · 7eecec7d
      Markus Armbruster 提交于
      object_property_set_description() and
      object_class_property_set_description() fail only when property @name
      is not found.
      
      There are 85 calls of object_property_set_description() and
      object_class_property_set_description().  None of them can fail:
      
      * 84 immediately follow the creation of the property.
      
      * The one in spapr_rng_instance_init() refers to a property created in
        spapr_rng_class_init(), from spapr_rng_properties[].
      
      Every one of them still gets to decide what to pass for @errp.
      
      51 calls pass &error_abort, 32 calls pass NULL, one receives the error
      and propagates it to &error_abort, and one propagates it to
      &error_fatal.  I'm actually surprised none of them violates the Error
      API.
      
      What are we gaining by letting callers handle the "property not found"
      error?  Use when the property is not known to exist is simpler: you
      don't have to guard the call with a check.  We haven't found such a
      use in 5+ years.  Until we do, let's make life a bit simpler and drop
      the @errp parameter.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-8-armbru@redhat.com>
      [One semantic rebase conflict resolved]
      7eecec7d
    • M
      qom: Drop convenience method object_property_get_uint16List() · 44a17fe0
      Markus Armbruster 提交于
      qom/object.c provides object_property_get_TYPE() and
      object_property_set_TYPE() for a number of common types.  These are
      all convenience wrappers around object_property_get_qobject() and
      object_property_set_qobject().
      
      Except for object_property_get_uint16List(), which is unusual in two ways:
      
      * It bypasses object_property_get_qobject().  Fixable; the previous
        commit did it for object_property_get_enum())
      
      * It stores the value through a parameter.  Its contract claims it
        returns the value, like the other functions do.  Also fixable.
      
      Fixing is not worthwhile, though: object_property_get_uint16List() has
      seen exactly one user in six years.
      
      Convert the lone user to do its job with the generic
      object_property_get_qobject(), and drop object_property_get_uint16List().
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <20200505152926.18877-6-armbru@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      [Commit message typo fixed]
      44a17fe0
    • M
      qom: Clean up inconsistent use of gchar * vs. char * · ddfb0baa
      Markus Armbruster 提交于
      Uses of gchar * in qom/object.h:
      
      * ObjectProperty member @name
      
        Functions that take a property name argument all use char *.  Change
        the member to match.
      
      * ObjectProperty member @type
      
        Functions that take a property type argument or return it all use
        char *.  Change the member to match.
      
      * ObjectProperty member @description
      
        Functions that take a property description argument all use char *.
        Change the member to match.
      
      * object_resolve_path_component() parameter @part
      
        Path components are property names.  Most callers pass char *
        arguments.  Change the parameter to match.  Adjust the few callers
        that pass gchar * to pass char *.
      
      * Return value of object_get_canonical_path_component(),
        object_get_canonical_path()
      
        Most callers convert their return values right back to char *.
        Change the return value to match.  Adjust the few callers where that
        would add a conversion to gchar * to use char * instead.
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Reviewed-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
      Message-Id: <20200505152926.18877-3-armbru@redhat.com>
      ddfb0baa
  2. 14 5月, 2020 17 次提交
  3. 12 5月, 2020 1 次提交
  4. 11 5月, 2020 7 次提交
  5. 07 5月, 2020 6 次提交
    • D
      spapr_nvdimm: Tweak error messages · 6c0f0cb3
      David Gibson 提交于
      The restrictions here (which are checked at pre-plug time) are PAPR
      specific, rather than being inherent to the NVDIMM devices.  Adjust the
      error messages to be clearer about this.
      Signed-off-by: NDavid Gibson <david@gibson.dropbear.id.au>
      6c0f0cb3
    • D
      spapr_nvdimm.c: make 'label-size' mandatory · 70fc9cb0
      Daniel Henrique Barboza 提交于
      The pseries machine does not support NVDIMM modules without label.
      Attempting to do so, even if the overall block size is aligned with
      256MB, will seg fault the guest kernel during NVDIMM probe. This
      can be avoided by forcing 'label-size' to always be present for
      sPAPR NVDIMMs.
      
      The verification was put before the alignment check because the
      presence of label-size affects the alignment calculation, so
      it's not optimal to warn the user about an alignment error,
      then about the lack of label-size, then about a new alignment
      error when the user sets a label-size.
      Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
      Message-Id: <20200413203628.31636-1-danielhb413@gmail.com>
      Signed-off-by: NDavid Gibson <david@gibson.dropbear.id.au>
      70fc9cb0
    • D
      spapr: Don't allow unplug of NVLink2 devices · 05af7c77
      David Gibson 提交于
      Currently, we can't properly handle unplug of NVLink2 devices, because we
      don't have code to tear down their special memory resources.  There's not
      a lot of impetus to implement that: since hardware NVLink2 devices can't
      be hot unplugged, the guest side drivers don't usually support unplug
      anyway.
      
      Therefore, simply prevent unplug of NVLink2 devices.
      Signed-off-by: NDavid Gibson <david@gibson.dropbear.id.au>
      Reviewed-by: NAlexey Kardashevskiy <aik@ozlabs.ru>
      05af7c77
    • G
      spapr: Drop CAS reboot flag · 087820e3
      Greg Kurz 提交于
      The CAS reboot flag is false by default and all the locations that
      could set it to true have been dropped. This means that all code
      blocks depending on the flag being set is dead code and the other
      code blocks should be executed always.
      
      Just do that and drop the now uneeded CAS reboot flag. Fix a
      comment on the way to make checkpatch happy.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Message-Id: <158514994893.478799.11772512888322840990.stgit@bahia.lan>
      Signed-off-by: NDavid Gibson <david@gibson.dropbear.id.au>
      087820e3
    • A
      spapr/cas: Separate CAS handling from rebuilding the FDT · 91067db1
      Alexey Kardashevskiy 提交于
      At the moment "ibm,client-architecture-support" ("CAS") is implemented
      in SLOF and QEMU assists via the custom H_CAS hypercall which copies
      an updated flatten device tree (FDT) blob to the SLOF memory which
      it then uses to update its internal tree.
      
      When we enable the OpenFirmware client interface in QEMU, we won't need
      to copy the FDT to the guest as the client is expected to fetch
      the device tree using the client interface.
      
      This moves FDT rebuild out to a separate helper which is going to be
      called from the "ibm,client-architecture-support" handler and leaves
      writing FDT to the guest in the H_CAS handler.
      
      This should not cause any behavioral change.
      Signed-off-by: NAlexey Kardashevskiy <aik@ozlabs.ru>
      Message-Id: <20200310050733.29805-3-aik@ozlabs.ru>
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Message-Id: <158514994229.478799.2178881312094922324.stgit@bahia.lan>
      Signed-off-by: NDavid Gibson <david@gibson.dropbear.id.au>
      91067db1
    • G
      spapr: Simplify selection of radix/hash during CAS · b4b83312
      Greg Kurz 提交于
      The guest can select the MMU mode by setting bits 0-1 of byte 24
      in OV5 to to 0b00 for hash or 0b01 for radix. As required by the
      architecture, we terminate the boot process if any other value
      is found there.
      
      The usual way to negotiate features in OV5 is basically ANDing
      the bitfield provided by the guest and the bitfield of features
      supported by QEMU, previously populated at machine init.
      
      For some not documented reason, MMU is treated differently : bit 1
      of byte 24 (the radix/hash bit) is cleared from the guest OV5 and
      explicitely set in the final negotiated OV5 if radix was requested.
      
      Since the only expected input from the guest is the radix/hash bit
      being set or not, it seems more appropriate to handle this like we
      do for XIVE.
      
      Set the radix bit in spapr->ov5 at machine init if it has a chance
      to work (ie. power9, either TCG or a radix capable KVM) and rely
      exclusively on spapr_ovec_intersect() to set the radix bit in
      spapr->ov5_cas.
      Signed-off-by: NGreg Kurz <groug@kaod.org>
      Message-Id: <158514993621.478799.4204740354545734293.stgit@bahia.lan>
      Signed-off-by: NDavid Gibson <david@gibson.dropbear.id.au>
      b4b83312