Re: [Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Philippe Mathieu-Daudé]

On 11/17/2017 10:42 AM, Mark Cave-Ayland wrote:
> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
> should be contained within the PCI bridge itself.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  hw/sparc64/sun4u.c |7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 394b7d6..63b4aaa 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -84,6 +84,7 @@ typedef struct EbusState {
>  /*< private >*/
>  PCIDevice parent_obj;
>  
> +ISABus *isa_bus;
>  MemoryRegion bar0;
>  MemoryRegion bar1;
>  } EbusState;
> @@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error 
> **errp)
>  {
>  EbusState *s = EBUS(pci_dev);
>  
> -if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
> - pci_address_space_io(pci_dev), errp)) {
> +s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
> + pci_address_space_io(pci_dev), errp);
> +if (!s->isa_bus) {
> +error_setg(errp, "unable to instantiate EBUS ISA bus");
>  return;
>  }
>  
> 

Re: [Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 4:46 PM, Mark Cave-Ayland
 wrote:
> On 17/11/17 14:53, Artyom Tarasenko wrote:
>
>> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
>>  wrote:
>>> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
>>> should be contained within the PCI bridge itself.
>>
>> While it's like that on the Sabre chipset, the Spitfire chipset (which
>> I hope to add at some point) has the EBus, but no PCI, so maybe it's
>> better to model it separately.
>> On the other hand, the Spitfire has different EBus devices
>> (particularly different type of the serial ports), so I'm not sure.
>
> Oh I didn't realise you had more plans in this area :)  Any idea when
> you'll be able to work on the them?

After I make AIX boot. :-)

> TBH as you probably already know,
> even the patchset in its current form with the ISA bus encapsulation is
> so much better than what is already there, so I'd prefer to merge it and
> help you work through any problems later unless you feel particularly
> strongly?

Ok, let's do it.

Reviewed-by: Artyom Tarasenko 

-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Mark Cave-Ayland]

On 17/11/17 14:53, Artyom Tarasenko wrote:

> On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
>  wrote:
>> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
>> should be contained within the PCI bridge itself.
> 
> While it's like that on the Sabre chipset, the Spitfire chipset (which
> I hope to add at some point) has the EBus, but no PCI, so maybe it's
> better to model it separately.
> On the other hand, the Spitfire has different EBus devices
> (particularly different type of the serial ports), so I'm not sure.

Oh I didn't realise you had more plans in this area :)  Any idea when
you'll be able to work on the them? TBH as you probably already know,
even the patchset in its current form with the ISA bus encapsulation is
so much better than what is already there, so I'd prefer to merge it and
help you work through any problems later unless you feel particularly
strongly?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Artyom Tarasenko]

On Fri, Nov 17, 2017 at 2:42 PM, Mark Cave-Ayland
 wrote:
> Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
> should be contained within the PCI bridge itself.

While it's like that on the Sabre chipset, the Spitfire chipset (which
I hope to add at some point) has the EBus, but no PCI, so maybe it's
better to model it separately.
On the other hand, the Spitfire has different EBus devices
(particularly different type of the serial ports), so I'm not sure.

> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/sparc64/sun4u.c |7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 394b7d6..63b4aaa 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -84,6 +84,7 @@ typedef struct EbusState {
>  /*< private >*/
>  PCIDevice parent_obj;
>
> +ISABus *isa_bus;
>  MemoryRegion bar0;
>  MemoryRegion bar1;
>  } EbusState;
> @@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error 
> **errp)
>  {
>  EbusState *s = EBUS(pci_dev);
>
> -if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
> - pci_address_space_io(pci_dev), errp)) {
> +s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
> + pci_address_space_io(pci_dev), errp);
> +if (!s->isa_bus) {
> +error_setg(errp, "unable to instantiate EBUS ISA bus");
>  return;
>  }
>
> --
> 1.7.10.4
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

[Qemu-devel] [PATCH 03/15] sun4u: move ISABus inside of EBusState

[Mark Cave-Ayland]

Since the EBus is effectively a PCI-ISA bridge then the underlying ISA bus
should be contained within the PCI bridge itself.

Signed-off-by: Mark Cave-Ayland 
---
 hw/sparc64/sun4u.c |7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 394b7d6..63b4aaa 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -84,6 +84,7 @@ typedef struct EbusState {
 /*< private >*/
 PCIDevice parent_obj;
 
+ISABus *isa_bus;
 MemoryRegion bar0;
 MemoryRegion bar1;
 } EbusState;
@@ -245,8 +246,10 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
 {
 EbusState *s = EBUS(pci_dev);
 
-if (!isa_bus_new(DEVICE(pci_dev), get_system_memory(),
- pci_address_space_io(pci_dev), errp)) {
+s->isa_bus = isa_bus_new(DEVICE(pci_dev), get_system_memory(),
+ pci_address_space_io(pci_dev), errp);
+if (!s->isa_bus) {
+error_setg(errp, "unable to instantiate EBUS ISA bus");
 return;
 }
 
-- 
1.7.10.4