Re: [Qemu-devel] [PATCH 2/2] hw/ppc/spapr: Use XHCI as host controller for new spapr machines

[Alexander Graf <agraf@suse.de>]

On 27.11.15 10:32, Thomas Huth wrote:
> The OHCI has some bugs and performance issues, so for newer
> machines it's preferable to use XHCI instead.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/ppc/spapr.c         | 15 ++++++++++++++-
>  include/hw/ppc/spapr.h |  3 ++-
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 10b7c35..f6e6888 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1914,7 +1914,11 @@ static void ppc_spapr_init(MachineState *machine)
>      }
>  
>      if (machine->usb) {
> -        pci_create_simple(phb->bus, -1, "pci-ohci");
> +        if (smc->use_ohci_by_default) {
> +            pci_create_simple(phb->bus, -1, "pci-ohci");
> +        } else {
> +            pci_create_simple(phb->bus, -1, "nec-usb-xhci");
> +        }
>  
>          if (spapr->has_graphics) {
>              USBBus *usb_bus = usb_bus_find(-1);
> @@ -2364,9 +2368,11 @@ static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>          SPAPR_COMPAT_2_1
>          { /* end of list */ }
>      };
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>  
>      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
>      mc->compat_props = compat_props;
> +    smc->use_ohci_by_default = true;
>  }
>  
>  static const TypeInfo spapr_machine_2_1_info = {
> @@ -2383,9 +2389,11 @@ static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>          { /* end of list */ }
>      };
>      MachineClass *mc = MACHINE_CLASS(oc);
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>  
>      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.2";
>      mc->compat_props = compat_props;
> +    smc->use_ohci_by_default = true;
>  }
>  
>  static const TypeInfo spapr_machine_2_2_info = {
> @@ -2402,9 +2410,11 @@ static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>          { /* end of list */ }
>      };
>      MachineClass *mc = MACHINE_CLASS(oc);
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>  
>      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
>      mc->compat_props = compat_props;
> +    smc->use_ohci_by_default = true;
>  }
>  
>  static const TypeInfo spapr_machine_2_3_info = {
> @@ -2421,9 +2431,11 @@ static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
>          { /* end of list */ }
>      };
>      MachineClass *mc = MACHINE_CLASS(oc);
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>  
>      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
>      mc->compat_props = compat_props;
> +    smc->use_ohci_by_default = true;
>  }
>  
>  static const TypeInfo spapr_machine_2_4_info = {
> @@ -2440,6 +2452,7 @@ static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
>      mc->name = "pseries-2.5";
>      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
>      smc->dr_lmb_enabled = true;
> +    smc->use_ohci_by_default = true;
>  }
>  
>  static const TypeInfo spapr_machine_2_5_info = {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 5baa906..53af76a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -35,7 +35,8 @@ struct sPAPRMachineClass {
>      MachineClass parent_class;
>  
>      /*< public >*/
> -    bool dr_lmb_enabled; /* enable dynamic-reconfig/hotplug of LMBs */
> +    bool dr_lmb_enabled;       /* enable dynamic-reconfig/hotplug of LMBs */
> +    bool use_ohci_by_default;  /* use USB-OHCI instead of XHCI */

I think with the x86 scheme of always calling the predecessor quirk
functions, we're better off with a flag saying "use_xhci_by_default"
that gets set from 2.6 onwards.

I also do agree that switching to xhci is the right path forward. I
think the main reason we didn't do it from the beginning was lack of
support in SLOF ;).


Alex