Re: [Qemu-devel] [PATCH v7 15/19] spapr/xive: enable XIVE MMIOs at reset

2018-12-11 Thread David Gibson 
On Tue, Dec 11, 2018 at 11:14:41AM +0100, Cédric Le Goater wrote:
> On 12/11/18 2:47 AM, David Gibson wrote:
> > On Sun, Dec 09, 2018 at 08:46:06PM +0100, Cédric Le Goater wrote:
> >> Depending on the interrupt mode chosen, enable or disable the XIVE
> >> MMIOs.
> >>
> >> Signed-off-by: Cédric Le Goater 
> >> ---
> >>  include/hw/ppc/spapr_xive.h | 1 +
> >>  hw/intc/spapr_xive.c| 9 +
> >>  hw/ppc/spapr_irq.c  | 8 
> >>  3 files changed, 18 insertions(+)
> >>
> >> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> >> index 7244a6231ce6..308afb61a666 100644
> >> --- a/include/hw/ppc/spapr_xive.h
> >> +++ b/include/hw/ppc/spapr_xive.h
> >> @@ -48,5 +48,6 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr);
> >>  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void 
> >> *fdt,
> >> uint32_t phandle);
> >>  void spapr_xive_reset_tctx(sPAPRXive *xive);
> >> +void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable);
> >>  
> >>  #endif /* PPC_SPAPR_XIVE_H */
> >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> >> index 560d8d031f74..c6dbb2e8cfc7 100644
> >> --- a/hw/intc/spapr_xive.c
> >> +++ b/hw/intc/spapr_xive.c
> >> @@ -179,6 +179,15 @@ static void spapr_xive_map_mmio(sPAPRXive *xive)
> >>  sysbus_mmio_map(SYS_BUS_DEVICE(xive), 2, xive->tm_base);
> >>  }
> >>  
> >> +void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable)
> > 
> > The logic looks fine, but I dislike this name - it's called
> > ..._enable() when it can also be used to disable.
> 
> ok. Let's call it spapr_xive_mmio_set_enabled() then

Sounds good.

> 
> C.
>  
> >> +{
> >> +memory_region_set_enabled(>source.esb_mmio, enable);
> >> +memory_region_set_enabled(>tm_mmio, enable);
> >> +
> >> +/* Disable the END ESBs until a guest OS makes use of them */
> >> +memory_region_set_enabled(>end_source.esb_mmio, false);
> >> +}
> >> +
> >>  /*
> >>   * When a Virtual Processor is scheduled to run on a HW thread, the
> >>   * hypervisor pushes its identifier in the OS CAM line. Emulate the
> >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> >> index b423cee30e2c..a8e50725397c 100644
> >> --- a/hw/ppc/spapr_irq.c
> >> +++ b/hw/ppc/spapr_irq.c
> >> @@ -217,6 +217,11 @@ static void spapr_irq_reset_xics(sPAPRMachineState 
> >> *spapr, Error **errp)
> >>  CPU_FOREACH(cs) {
> >>  spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
> >>  }
> >> +
> >> +/* Deactivate the XIVE MMIOs */
> >> +if (spapr->xive) {
> >> +spapr_xive_enable_mmio(spapr->xive, false);
> >> +}
> >>  }
> >>  
> >>  #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
> >> @@ -358,6 +363,9 @@ static void spapr_irq_reset_xive(sPAPRMachineState 
> >> *spapr, Error **errp)
> >>   * to come after the XiveTCTX reset handlers.
> >>   */
> >>  spapr_xive_reset_tctx(spapr->xive);
> >> +
> >> +/* Activate the XIVE MMIOs */
> >> +spapr_xive_enable_mmio(spapr->xive, true);
> >>  }
> >>  
> >>  /*
> > 
> 

-- 
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

Re: [Qemu-devel] [PATCH v7 15/19] spapr/xive: enable XIVE MMIOs at reset

2018-12-11 Thread Cédric Le Goater 
On 12/11/18 2:47 AM, David Gibson wrote:
> On Sun, Dec 09, 2018 at 08:46:06PM +0100, Cédric Le Goater wrote:
>> Depending on the interrupt mode chosen, enable or disable the XIVE
>> MMIOs.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  include/hw/ppc/spapr_xive.h | 1 +
>>  hw/intc/spapr_xive.c| 9 +
>>  hw/ppc/spapr_irq.c  | 8 
>>  3 files changed, 18 insertions(+)
>>
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index 7244a6231ce6..308afb61a666 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -48,5 +48,6 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr);
>>  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
>> uint32_t phandle);
>>  void spapr_xive_reset_tctx(sPAPRXive *xive);
>> +void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable);
>>  
>>  #endif /* PPC_SPAPR_XIVE_H */
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index 560d8d031f74..c6dbb2e8cfc7 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -179,6 +179,15 @@ static void spapr_xive_map_mmio(sPAPRXive *xive)
>>  sysbus_mmio_map(SYS_BUS_DEVICE(xive), 2, xive->tm_base);
>>  }
>>  
>> +void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable)
> 
> The logic looks fine, but I dislike this name - it's called
> ..._enable() when it can also be used to disable.

ok. Let's call it spapr_xive_mmio_set_enabled() then

C.
 
>> +{
>> +memory_region_set_enabled(>source.esb_mmio, enable);
>> +memory_region_set_enabled(>tm_mmio, enable);
>> +
>> +/* Disable the END ESBs until a guest OS makes use of them */
>> +memory_region_set_enabled(>end_source.esb_mmio, false);
>> +}
>> +
>>  /*
>>   * When a Virtual Processor is scheduled to run on a HW thread, the
>>   * hypervisor pushes its identifier in the OS CAM line. Emulate the
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index b423cee30e2c..a8e50725397c 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -217,6 +217,11 @@ static void spapr_irq_reset_xics(sPAPRMachineState 
>> *spapr, Error **errp)
>>  CPU_FOREACH(cs) {
>>  spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
>>  }
>> +
>> +/* Deactivate the XIVE MMIOs */
>> +if (spapr->xive) {
>> +spapr_xive_enable_mmio(spapr->xive, false);
>> +}
>>  }
>>  
>>  #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
>> @@ -358,6 +363,9 @@ static void spapr_irq_reset_xive(sPAPRMachineState 
>> *spapr, Error **errp)
>>   * to come after the XiveTCTX reset handlers.
>>   */
>>  spapr_xive_reset_tctx(spapr->xive);
>> +
>> +/* Activate the XIVE MMIOs */
>> +spapr_xive_enable_mmio(spapr->xive, true);
>>  }
>>  
>>  /*
>

Re: [Qemu-devel] [PATCH v7 15/19] spapr/xive: enable XIVE MMIOs at reset

2018-12-10 Thread David Gibson 
On Sun, Dec 09, 2018 at 08:46:06PM +0100, Cédric Le Goater wrote:
> Depending on the interrupt mode chosen, enable or disable the XIVE
> MMIOs.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/ppc/spapr_xive.h | 1 +
>  hw/intc/spapr_xive.c| 9 +
>  hw/ppc/spapr_irq.c  | 8 
>  3 files changed, 18 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 7244a6231ce6..308afb61a666 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -48,5 +48,6 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr);
>  void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
> uint32_t phandle);
>  void spapr_xive_reset_tctx(sPAPRXive *xive);
> +void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable);
>  
>  #endif /* PPC_SPAPR_XIVE_H */
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 560d8d031f74..c6dbb2e8cfc7 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -179,6 +179,15 @@ static void spapr_xive_map_mmio(sPAPRXive *xive)
>  sysbus_mmio_map(SYS_BUS_DEVICE(xive), 2, xive->tm_base);
>  }
>  
> +void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable)

The logic looks fine, but I dislike this name - it's called
..._enable() when it can also be used to disable.

> +{
> +memory_region_set_enabled(>source.esb_mmio, enable);
> +memory_region_set_enabled(>tm_mmio, enable);
> +
> +/* Disable the END ESBs until a guest OS makes use of them */
> +memory_region_set_enabled(>end_source.esb_mmio, false);
> +}
> +
>  /*
>   * When a Virtual Processor is scheduled to run on a HW thread, the
>   * hypervisor pushes its identifier in the OS CAM line. Emulate the
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index b423cee30e2c..a8e50725397c 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -217,6 +217,11 @@ static void spapr_irq_reset_xics(sPAPRMachineState 
> *spapr, Error **errp)
>  CPU_FOREACH(cs) {
>  spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
>  }
> +
> +/* Deactivate the XIVE MMIOs */
> +if (spapr->xive) {
> +spapr_xive_enable_mmio(spapr->xive, false);
> +}
>  }
>  
>  #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
> @@ -358,6 +363,9 @@ static void spapr_irq_reset_xive(sPAPRMachineState 
> *spapr, Error **errp)
>   * to come after the XiveTCTX reset handlers.
>   */
>  spapr_xive_reset_tctx(spapr->xive);
> +
> +/* Activate the XIVE MMIOs */
> +spapr_xive_enable_mmio(spapr->xive, true);
>  }
>  
>  /*

-- 
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

[Qemu-devel] [PATCH v7 15/19] spapr/xive: enable XIVE MMIOs at reset

2018-12-09 Thread Cédric Le Goater 
Depending on the interrupt mode chosen, enable or disable the XIVE
MMIOs.

Signed-off-by: Cédric Le Goater 
---
 include/hw/ppc/spapr_xive.h | 1 +
 hw/intc/spapr_xive.c| 9 +
 hw/ppc/spapr_irq.c  | 8 
 3 files changed, 18 insertions(+)

diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
index 7244a6231ce6..308afb61a666 100644
--- a/include/hw/ppc/spapr_xive.h
+++ b/include/hw/ppc/spapr_xive.h
@@ -48,5 +48,6 @@ void spapr_xive_hcall_init(sPAPRMachineState *spapr);
 void spapr_dt_xive(sPAPRMachineState *spapr, uint32_t nr_servers, void *fdt,
uint32_t phandle);
 void spapr_xive_reset_tctx(sPAPRXive *xive);
+void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable);
 
 #endif /* PPC_SPAPR_XIVE_H */
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 560d8d031f74..c6dbb2e8cfc7 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -179,6 +179,15 @@ static void spapr_xive_map_mmio(sPAPRXive *xive)
 sysbus_mmio_map(SYS_BUS_DEVICE(xive), 2, xive->tm_base);
 }
 
+void spapr_xive_enable_mmio(sPAPRXive *xive, bool enable)
+{
+memory_region_set_enabled(>source.esb_mmio, enable);
+memory_region_set_enabled(>tm_mmio, enable);
+
+/* Disable the END ESBs until a guest OS makes use of them */
+memory_region_set_enabled(>end_source.esb_mmio, false);
+}
+
 /*
  * When a Virtual Processor is scheduled to run on a HW thread, the
  * hypervisor pushes its identifier in the OS CAM line. Emulate the
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index b423cee30e2c..a8e50725397c 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -217,6 +217,11 @@ static void spapr_irq_reset_xics(sPAPRMachineState *spapr, 
Error **errp)
 CPU_FOREACH(cs) {
 spapr_cpu_core_set_intc(POWERPC_CPU(cs), spapr->icp_type);
 }
+
+/* Deactivate the XIVE MMIOs */
+if (spapr->xive) {
+spapr_xive_enable_mmio(spapr->xive, false);
+}
 }
 
 #define SPAPR_IRQ_XICS_NR_IRQS 0x1000
@@ -358,6 +363,9 @@ static void spapr_irq_reset_xive(sPAPRMachineState *spapr, 
Error **errp)
  * to come after the XiveTCTX reset handlers.
  */
 spapr_xive_reset_tctx(spapr->xive);
+
+/* Activate the XIVE MMIOs */
+spapr_xive_enable_mmio(spapr->xive, true);
 }
 
 /*
-- 
2.17.2