Re: [Qemu-devel] Re: Extending qemu_irq for reset signals
> I implemented reset that way with qemu_irq, the result survives some > quick tests. I'd probably make it take notice of the level parameter, and lower the line immediately after raising it. Future implementations of qemu_irq may cache the value internally. Other than that, looks ok, though your patch doesn't apply against current cvs. Paul
Re: [Qemu-devel] Re: Extending qemu_irq for reset signals
On 8/15/07, Blue Swirl <[EMAIL PROTECTED]> wrote:
> On 8/15/07, Paul Brook <[EMAIL PROTECTED]> wrote:
> > On Wednesday 15 August 2007, Blue Swirl wrote:
> > > On 8/15/07, Paul Brook <[EMAIL PROTECTED]> wrote:
> > > > On Wednesday 15 August 2007, Paul Brook wrote:
> > > > > > Okay, more explaining. This is the case where I'd want to use the
> > > > > > signal: DMA controller ("upstream") can reset the slave device (ESP
> > > > > > or Lance). DMA controller is created first and I also want to
> > > > > > allocate reset signals at that point. Later when ESP is created, it
> > > > > > should be possible to put ESP reset function and opaque data to the
> > > > > > signal given but this is not possible with current API. Currently
> > > > > > the
> > > > > > DMA data would be passed to qemu_allocate_irqs.
> > > > >
> > > > > Ah, I see. The problem here is that you've got a cyclic dependency.
> > > > > For
> > > > > DMA operations the ESP is in charge, so it makes sense to create the
> > > > > subservient DMA device first. For the reset signals the DMA controller
> > > > > is in charge so ideally you create the ESP device first. Because the
> > > > > DMA interface is most complicated, it's probably takes precedence.
> > > > >
> > > > > I think you need to modify or use sparc32_dma_set_reset_data to take a
> > > > > qemu_irq rather than a callback and opaque argument. Alternatively you
> > > > > can move things around a bit and have the sun4m code do something
> > > > > similar. i.e. the ESP and lance devices return the reset lines, then
> > > > > the sun4m code pokes into the DMA device state.
> > > >
> > > > Oh, or you can pass a pointer to a qemu_irq from the DMA to the ESP and
> > > > have the ESP poke its reset object in there that way.
> > >
> > > That's what I had in mind. Should I just extend the API for example with
> > > /* Change the callback function and/or data */
> > > void qemu_irq_change(qemu_irq irq, qemu_irq_handler handler,
> > > void *opaque);
> >
> > I'm not so keen on that. It breaks the nice property of having the IRQ lines
> > owned by the receiver. I was thinking something like:
> >
> > struct DMAState {
> > qemu_irq device_reset;
> > };
> > qemu_irq *dma_init()
> > {
> > struct DMAState * d = malloc();
> > return &d->device_reset;
> > }
> >
> > void esp_init(qemu_irq *resetp)
> > {
> > ESPState *d = malloc();
> > *reset = *qemu_irq_alloc(d, 1);
> > }
> >
> > void sun4m_init()
> > {
> > qemu_irq *p = dma_init();
> > esp_init(p);
> > }
>
> Yes, that would work. I wasn't too happy about the change function either.
>
I implemented reset that way with qemu_irq, the result survives some
quick tests.
Index: qemu/hw/esp.c
===
--- qemu.orig/hw/esp.c 2007-08-15 18:58:02.0 +
+++ qemu/hw/esp.c 2007-08-15 19:22:38.0 +
@@ -344,6 +344,11 @@
s->do_cmd = 0;
}
+static void parent_esp_reset(void *opaque, int irq, int level)
+{
+esp_reset(opaque);
+}
+
static uint32_t esp_mem_readb(void *opaque, target_phys_addr_t addr)
{
ESPState *s = opaque;
@@ -569,7 +574,7 @@
}
void *esp_init(BlockDriverState **bd, target_phys_addr_t espaddr,
- void *dma_opaque, qemu_irq irq, qemu_dma *parent_dma)
+ qemu_irq irq, qemu_dma *parent_dma, qemu_irq *reset)
{
ESPState *s;
int esp_io_memory;
@@ -581,7 +586,6 @@
s->bd = bd;
s->irq = irq;
s->parent_dma = parent_dma;
-sparc32_dma_set_reset_data(dma_opaque, esp_reset, s);
esp_io_memory = cpu_register_io_memory(0, esp_mem_read, esp_mem_write, s);
cpu_register_physical_memory(espaddr, ESP_SIZE, esp_io_memory);
@@ -591,5 +595,7 @@
register_savevm("esp", espaddr, 3, esp_save, esp_load, s);
qemu_register_reset(esp_reset, s);
+*reset = *qemu_allocate_irqs(parent_esp_reset, s, 1);
+
return s;
}
Index: qemu/hw/sun4m.c
===
--- qemu.orig/hw/sun4m.c 2007-08-15 18:58:02.0 +
+++ qemu/hw/sun4m.c 2007-08-15 19:05:57.0 +
@@ -322,6 +322,7 @@
qemu_irq *cpu_irqs[MAX_CPUS], *slavio_irq, *slavio_cpu_irq,
*espdma_irq, *ledma_irq;
qemu_dma *physical_dma, *dvma, *esp_dvma, *le_dvma;
+qemu_irq *esp_reset, *le_reset;
/* init CPUs */
sparc_find_by_name(cpu_model, &def);
@@ -362,11 +363,11 @@
hwdef->clock_irq);
espdma = sparc32_dma_init(hwdef->dma_base, slavio_irq[hwdef->esp_irq],
- &espdma_irq, dvma, &esp_dvma, 1);
+ &espdma_irq, dvma, &esp_dvma, 1, &esp_reset);
ledma = sparc32_dma_init(hwdef->dma_base + 16ULL,
slavio_irq[hwdef->le_irq], &ledma_irq, dvma,
- &le_dvma, 0);
+ &le_dvma, 0, &le_reset);
if (graphic_depth != 8 && graphic_depth != 24) {
Re: [Qemu-devel] Re: Extending qemu_irq for reset signals
On 8/15/07, Paul Brook <[EMAIL PROTECTED]> wrote:
> On Wednesday 15 August 2007, Blue Swirl wrote:
> > On 8/15/07, Paul Brook <[EMAIL PROTECTED]> wrote:
> > > On Wednesday 15 August 2007, Paul Brook wrote:
> > > > > Okay, more explaining. This is the case where I'd want to use the
> > > > > signal: DMA controller ("upstream") can reset the slave device (ESP
> > > > > or Lance). DMA controller is created first and I also want to
> > > > > allocate reset signals at that point. Later when ESP is created, it
> > > > > should be possible to put ESP reset function and opaque data to the
> > > > > signal given but this is not possible with current API. Currently the
> > > > > DMA data would be passed to qemu_allocate_irqs.
> > > >
> > > > Ah, I see. The problem here is that you've got a cyclic dependency. For
> > > > DMA operations the ESP is in charge, so it makes sense to create the
> > > > subservient DMA device first. For the reset signals the DMA controller
> > > > is in charge so ideally you create the ESP device first. Because the
> > > > DMA interface is most complicated, it's probably takes precedence.
> > > >
> > > > I think you need to modify or use sparc32_dma_set_reset_data to take a
> > > > qemu_irq rather than a callback and opaque argument. Alternatively you
> > > > can move things around a bit and have the sun4m code do something
> > > > similar. i.e. the ESP and lance devices return the reset lines, then
> > > > the sun4m code pokes into the DMA device state.
> > >
> > > Oh, or you can pass a pointer to a qemu_irq from the DMA to the ESP and
> > > have the ESP poke its reset object in there that way.
> >
> > That's what I had in mind. Should I just extend the API for example with
> > /* Change the callback function and/or data */
> > void qemu_irq_change(qemu_irq irq, qemu_irq_handler handler,
> > void *opaque);
>
> I'm not so keen on that. It breaks the nice property of having the IRQ lines
> owned by the receiver. I was thinking something like:
>
> struct DMAState {
> qemu_irq device_reset;
> };
> qemu_irq *dma_init()
> {
> struct DMAState * d = malloc();
> return &d->device_reset;
> }
>
> void esp_init(qemu_irq *resetp)
> {
> ESPState *d = malloc();
> *reset = *qemu_irq_alloc(d, 1);
> }
>
> void sun4m_init()
> {
> qemu_irq *p = dma_init();
> esp_init(p);
> }
Yes, that would work. I wasn't too happy about the change function either.
[Qemu-devel] Re: Extending qemu_irq for reset signals
Preferably also the callback function type should be QEMUResetHandler, not the slightly different qemu_irq_handler.
[Qemu-devel] Re: Extending qemu_irq for reset signals
On Wednesday 15 August 2007, Blue Swirl wrote:
> On 8/15/07, Paul Brook <[EMAIL PROTECTED]> wrote:
> > On Wednesday 15 August 2007, Paul Brook wrote:
> > > > Okay, more explaining. This is the case where I'd want to use the
> > > > signal: DMA controller ("upstream") can reset the slave device (ESP
> > > > or Lance). DMA controller is created first and I also want to
> > > > allocate reset signals at that point. Later when ESP is created, it
> > > > should be possible to put ESP reset function and opaque data to the
> > > > signal given but this is not possible with current API. Currently the
> > > > DMA data would be passed to qemu_allocate_irqs.
> > >
> > > Ah, I see. The problem here is that you've got a cyclic dependency. For
> > > DMA operations the ESP is in charge, so it makes sense to create the
> > > subservient DMA device first. For the reset signals the DMA controller
> > > is in charge so ideally you create the ESP device first. Because the
> > > DMA interface is most complicated, it's probably takes precedence.
> > >
> > > I think you need to modify or use sparc32_dma_set_reset_data to take a
> > > qemu_irq rather than a callback and opaque argument. Alternatively you
> > > can move things around a bit and have the sun4m code do something
> > > similar. i.e. the ESP and lance devices return the reset lines, then
> > > the sun4m code pokes into the DMA device state.
> >
> > Oh, or you can pass a pointer to a qemu_irq from the DMA to the ESP and
> > have the ESP poke its reset object in there that way.
>
> That's what I had in mind. Should I just extend the API for example with
> /* Change the callback function and/or data */
> void qemu_irq_change(qemu_irq irq, qemu_irq_handler handler,
> void *opaque);
I'm not so keen on that. It breaks the nice property of having the IRQ lines
owned by the receiver. I was thinking something like:
struct DMAState {
qemu_irq device_reset;
};
qemu_irq *dma_init()
{
struct DMAState * d = malloc();
return &d->device_reset;
}
void esp_init(qemu_irq *resetp)
{
ESPState *d = malloc();
*reset = *qemu_irq_alloc(d, 1);
}
void sun4m_init()
{
qemu_irq *p = dma_init();
esp_init(p);
}
Paul
[Qemu-devel] Re: Extending qemu_irq for reset signals
On 8/15/07, Paul Brook <[EMAIL PROTECTED]> wrote:
> On Wednesday 15 August 2007, Paul Brook wrote:
> > > Okay, more explaining. This is the case where I'd want to use the
> > > signal: DMA controller ("upstream") can reset the slave device (ESP or
> > > Lance). DMA controller is created first and I also want to allocate
> > > reset signals at that point. Later when ESP is created, it should be
> > > possible to put ESP reset function and opaque data to the signal given
> > > but this is not possible with current API. Currently the DMA data
> > > would be passed to qemu_allocate_irqs.
> >
> > Ah, I see. The problem here is that you've got a cyclic dependency. For DMA
> > operations the ESP is in charge, so it makes sense to create the
> > subservient DMA device first. For the reset signals the DMA controller is
> > in charge so ideally you create the ESP device first. Because the DMA
> > interface is most complicated, it's probably takes precedence.
> >
> > I think you need to modify or use sparc32_dma_set_reset_data to take a
> > qemu_irq rather than a callback and opaque argument. Alternatively you can
> > move things around a bit and have the sun4m code do something similar. i.e.
> > the ESP and lance devices return the reset lines, then the sun4m code pokes
> > into the DMA device state.
>
> Oh, or you can pass a pointer to a qemu_irq from the DMA to the ESP and have
> the ESP poke its reset object in there that way.
That's what I had in mind. Should I just extend the API for example with
/* Change the callback function and/or data */
void qemu_irq_change(qemu_irq irq, qemu_irq_handler handler,
void *opaque);
?
There is a small problem that DMA may not assert the reset signal
before ESP gets the data in place. Usually devices perform reset when
created.
[Qemu-devel] Re: Extending qemu_irq for reset signals
On Wednesday 15 August 2007, Paul Brook wrote:
> > Okay, more explaining. This is the case where I'd want to use the
> > signal: DMA controller ("upstream") can reset the slave device (ESP or
> > Lance). DMA controller is created first and I also want to allocate
> > reset signals at that point. Later when ESP is created, it should be
> > possible to put ESP reset function and opaque data to the signal given
> > but this is not possible with current API. Currently the DMA data
> > would be passed to qemu_allocate_irqs.
>
> Ah, I see. The problem here is that you've got a cyclic dependency. For DMA
> operations the ESP is in charge, so it makes sense to create the
> subservient DMA device first. For the reset signals the DMA controller is
> in charge so ideally you create the ESP device first. Because the DMA
> interface is most complicated, it's probably takes precedence.
>
> I think you need to modify or use sparc32_dma_set_reset_data to take a
> qemu_irq rather than a callback and opaque argument. Alternatively you can
> move things around a bit and have the sun4m code do something similar. i.e.
> the ESP and lance devices return the reset lines, then the sun4m code pokes
> into the DMA device state.
Oh, or you can pass a pointer to a qemu_irq from the DMA to the ESP and have
the ESP poke its reset object in there that way.
Paul
[Qemu-devel] Re: Extending qemu_irq for reset signals
> Okay, more explaining. This is the case where I'd want to use the
> signal: DMA controller ("upstream") can reset the slave device (ESP or
> Lance). DMA controller is created first and I also want to allocate
> reset signals at that point. Later when ESP is created, it should be
> possible to put ESP reset function and opaque data to the signal given
> but this is not possible with current API. Currently the DMA data
> would be passed to qemu_allocate_irqs.
Ah, I see. The problem here is that you've got a cyclic dependency. For DMA
operations the ESP is in charge, so it makes sense to create the subservient
DMA device first. For the reset signals the DMA controller is in charge so
ideally you create the ESP device first. Because the DMA interface is most
complicated, it's probably takes precedence.
I think you need to modify or use sparc32_dma_set_reset_data to take a
qemu_irq rather than a callback and opaque argument. Alternatively you can
move things around a bit and have the sun4m code do something similar. i.e.
the ESP and lance devices return the reset lines, then the sun4m code pokes
into the DMA device state.
Paul
[Qemu-devel] Re: Extending qemu_irq for reset signals
On 8/15/07, Paul Brook <[EMAIL PROTECTED]> wrote:
> On Wednesday 15 August 2007, Blue Swirl wrote:
> > Hi,
> >
> > I'd like to use similar mechanism as qemu_irq as reset signal for
> > devices. The difference is that the opaque data and callback are not
> > specified at the time of creating the signal at upstream device, but
> > when the receiving device is created. This does not fit current
> > qemu_irq model.
>
> Either your confused, or not expressing what you want very well. What you
> describe is how qemu_irq works.
>
> The "upstream device" and "receiving device" are the same thing.
Okay, more explaining. This is the case where I'd want to use the
signal: DMA controller ("upstream") can reset the slave device (ESP or
Lance). DMA controller is created first and I also want to allocate
reset signals at that point. Later when ESP is created, it should be
possible to put ESP reset function and opaque data to the signal given
but this is not possible with current API. Currently the DMA data
would be passed to qemu_allocate_irqs.
> The whole point of qemu_irq is to be able to send a single-bit signal without
> having to know or care where it's going. ie. when creating the board/cpu, you
> create the qemu_irq object with appropriate parameters. Then the device
> initiating the reset uses that qemu_irq object.
>
> If the device initiating the reset needs to pass additional information, then
> you're no longer dealing with a single bit of state, so I don't think
> qemu_irq is really appropriate.
Agreed. No data needs to be passed in my case and the qemu_irq model
works except for the API.
[Qemu-devel] Re: Extending qemu_irq for reset signals
On Wednesday 15 August 2007, Blue Swirl wrote: > Hi, > > I'd like to use similar mechanism as qemu_irq as reset signal for > devices. The difference is that the opaque data and callback are not > specified at the time of creating the signal at upstream device, but > when the receiving device is created. This does not fit current > qemu_irq model. Either your confused, or not expressing what you want very well. What you describe is how qemu_irq works. The "upstream device" and "receiving device" are the same thing. The whole point of qemu_irq is to be able to send a single-bit signal without having to know or care where it's going. ie. when creating the board/cpu, you create the qemu_irq object with appropriate parameters. Then the device initiating the reset uses that qemu_irq object. If the device initiating the reset needs to pass additional information, then you're no longer dealing with a single bit of state, so I don't think qemu_irq is really appropriate. Paul
