On 2012-09-04 13:37, Jan Kiszka wrote:
> On 2012-09-04 13:27, Julien Grall wrote:
>> On 09/04/2012 08:24 AM, Jan Kiszka wrote:
>>> On 2012-09-03 12:03, Julien Grall wrote:
>>>
>>>> This patch replaces all register_ioport* with the new memory API. It
>>>> permits
>>>> to use the new Memory stuff like listener.
>>>>
>>>> Signed-off-by: Julien Grall<julien.gr...@citrix.com>
>>>> ---
>>>> hw/acpi_piix4.c | 151
>>>> +++++++++++++++++++++++++++++++++++++++++++------------
>>>> 1 files changed, 119 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>>>> index 1ad45ce..527dfc1 100644
>>>> --- a/hw/acpi_piix4.c
>>>> +++ b/hw/acpi_piix4.c
>>>> @@ -41,8 +41,7 @@
>>>>
>>>> #define GPE_BASE 0xafe0
>>>> #define GPE_LEN 4
>>>> -#define PCI_UP_BASE 0xae00
>>>> -#define PCI_DOWN_BASE 0xae04
>>>> +#define PCI_BASE 0xae00
>>>> #define PCI_EJ_BASE 0xae08
>>>> #define PCI_RMV_BASE 0xae0c
>>>>
>>>> @@ -55,7 +54,8 @@ struct pci_status {
>>>>
>>>> typedef struct PIIX4PMState {
>>>> PCIDevice dev;
>>>> - IORange ioport;
>>>> + MemoryRegion pm_io;
>>>> + uint32_t pm_io_base;
>>>> ACPIREGS ar;
>>>>
>>>> APMState apm;
>>>> @@ -64,6 +64,11 @@ typedef struct PIIX4PMState {
>>>> uint32_t smb_io_base;
>>>>
>>>> MemoryRegion smb_io;
>>>> + MemoryRegion acpi_io;
>>>> + MemoryRegion acpi_hot_io;
>>>> + PortioList pci_hot_port_list;
>>>> + MemoryRegion pciej_hot_io;
>>>> + MemoryRegion pcirmv_hot_io;
>>>>
>>>> qemu_irq irq;
>>>> qemu_irq smi_irq;
>>>> @@ -110,10 +115,10 @@ static void pm_tmr_timer(ACPIREGS *ar)
>>>> pm_update_sci(s);
>>>> }
>>>>
>>>> -static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned
>>>> width,
>>>> - uint64_t val)
>>>> +static void pm_ioport_write(void *opaque, target_phys_addr_t addr,
>>>> + uint64_t val, unsigned width)
>>>> {
>>>> - PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>>>> + PIIX4PMState *s = opaque;
>>>>
>>>> if (width != 2) {
>>>> PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
>>>> @@ -139,11 +144,11 @@ static void pm_ioport_write(IORange *ioport,
>>>> uint64_t addr, unsigned width,
>>>> (unsigned int)val);
>>>> }
>>>>
>>>> -static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned
>>>> width,
>>>> - uint64_t *data)
>>>> +static uint64_t pm_ioport_read(void *opaque, target_phys_addr_t addr,
>>>> + unsigned width)
>>>> {
>>>> - PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>>>> - uint32_t val;
>>>> + PIIX4PMState *s = opaque;
>>>> + uint64_t val;
>>>>
>>>> switch(addr) {
>>>> case 0x00:
>>>> @@ -163,12 +168,18 @@ static void pm_ioport_read(IORange *ioport,
>>>> uint64_t addr, unsigned width,
>>>> break;
>>>> }
>>>> PIIX4_DPRINTF("PM readw port=0x%04x val=0x%04x\n", (unsigned
>>>> int)addr, val);
>>>> - *data = val;
>>>> +
>>>> + return val;
>>>> }
>>>>
>>>> -static const IORangeOps pm_iorange_ops = {
>>>> +static const MemoryRegionOps pm_io_ops = {
>>>> .read = pm_ioport_read,
>>>> .write = pm_ioport_write,
>>>> + .endianness = DEVICE_LITTLE_ENDIAN,
>>>> + .impl = {
>>>> + .min_access_size = 2,
>>>> + .max_access_size = 2,
>>>> + },
>>>> };
>>>>
>>>> static void apm_ctrl_changed(uint32_t val, void *arg)
>>>> @@ -185,7 +196,8 @@ static void apm_ctrl_changed(uint32_t val, void
>>>> *arg)
>>>> }
>>>> }
>>>>
>>>> -static void acpi_dbg_writel(void *opaque, uint32_t addr, uint32_t val)
>>>> +static void acpi_dbg_writel(void *opaque, target_phys_addr_t addr,
>>>> + uint64_t val, unsigned size)
>>>> {
>>>> PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val);
>>>> }
>>>> @@ -200,8 +212,18 @@ static void pm_io_space_update(PIIX4PMState *s)
>>>>
>>>> /* XXX: need to improve memory and ioport allocation */
>>>> PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
>>>> - iorange_init(&s->ioport,&pm_iorange_ops, pm_io_base, 64);
>>>> - ioport_register(&s->ioport);
>>>> +
>>>> + if (!s->pm_io_base) {
>>>> + memory_region_add_subregion(pci_address_space_io(&s->dev),
>>>> + pm_io_base,&s->pm_io);
>>>>
>>> This doesn't work reliably. Add the region in a disabled state (ie. the
>>> default) during init, only toggle enable and update the base here.
>>>
>> What is the default base address ? The datasheet
>> doesn't give default value. But the base address seems to
>> always be 0xb000. Is there any drawbacks to used it ?
>
> According to the spec
> (http://www.intel.com/assets/pdf/datasheet/290562.pdf), the reset value
> must be 0.
More precisely, the base is 0, the corresponding register is 1 after
reset (page 120, Power Management Base Address)).
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
