[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC 34/35] arm : acpi workarounds for firmware/linux dependencies



On 5 February 2015 at 23:18, Stefano Stabellini
<stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Wed, 4 Feb 2015, parth.dixit@xxxxxxxxxx wrote:
>> From: Parth Dixit <parth.dixit@xxxxxxxxxx>
>>
>> Some bugs are identified in edk2 and some of the functionality is not
>> yet merged. This patch contains workarounds for them
>
> A patch with a few workarounds left is OK, but you should explain
> exactly what they are and why you weren't able to solve the relative
> issues properly.
>
>
>> Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
>> ---
>>  xen/arch/arm/domain_build.c | 82 
>> ++++++++++++++++++++++++++++++++++++++++++++-
>>  xen/arch/arm/vgic.c         | 16 +++++++++
>>  xen/drivers/acpi/osl.c      |  7 ++--
>>  3 files changed, 102 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index a504064..a425ef4 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1234,7 +1234,87 @@ static int make_chosen_node(const struct domain *d, 
>> const struct kernel_info *ki
>>
>>      return res;
>>  }
>> +#define ACPI_UEFI_MEM_STUB
>> +
>> +#ifdef ACPI_UEFI_MEM_STUB
>> +enum{
>> +    IO_NET0,
>> +    IO_SREG,
>> +    IO_VIRT,
>> +    IO_SCT0,
>> +    IO_AAC0,
>> +    IO_MMC0,
>> +    IO_KMI0,
>> +    IO_KMI1,
>> +    IO_SER1,
>> +    IO_SER2,
>> +    IO_SER3,
>> +    IO_WDT0,
>> +    IO_TIM0,
>> +    IO_TIM2,
>> +    IO_RTC0,
>> +    IO_TIM3,
>> +    IO_TIM4,
>> +    IO_MAX
>> +};
>> +#define INIT_IO( dev,addr,size ) [dev] = {addr,size}
>> +
>> +static const u64 acpi_mmio_region[][IO_MAX]=
>> +    {
>> +        INIT_IO(IO_NET0,0x1a000000,(PAGE_SIZE*16) ),
>> +        INIT_IO(IO_SREG,0x1c010000,PAGE_SIZE),
>> +        INIT_IO(IO_VIRT,0x1c130000,PAGE_SIZE),
>> +        INIT_IO(IO_SCT0,0x1c020000,PAGE_SIZE),
>> +        INIT_IO(IO_AAC0,0x1c040000,PAGE_SIZE),
>> +        INIT_IO(IO_MMC0,0x1c050000,PAGE_SIZE),
>> +        INIT_IO(IO_KMI0,0x1c060000,PAGE_SIZE),
>> +        INIT_IO(IO_KMI1,0x1c070000,PAGE_SIZE),
>> +        INIT_IO(IO_SER1,0x1c0a0000,PAGE_SIZE),
>> +        INIT_IO(IO_SER2,0x1c0b0000,PAGE_SIZE),
>> +        INIT_IO(IO_SER3,0x1c0c0000,PAGE_SIZE),
>> +        INIT_IO(IO_WDT0,0x1c0f0000,PAGE_SIZE),
>> +        INIT_IO(IO_TIM0,0x1c110000,PAGE_SIZE),
>> +        INIT_IO(IO_TIM2,0x1c120000,PAGE_SIZE),
>> +        INIT_IO(IO_RTC0,0x1c170000,PAGE_SIZE),
>> +        INIT_IO(IO_TIM3,0x2a810000,(PAGE_SIZE*16) ),
>> +        INIT_IO(IO_TIM4,0x2a830000,(PAGE_SIZE*16) ),
>> +    };
>
> What is this?
This is the mmio map of fvp model as these regions are not described
in uefi firmware right now,
bug is already filed for it.
>
>> +int acpi_map_mmio(struct domain *d)
>> +{
>> +    int i,res;
>> +    u64 addr,size;
>> +
>> +    for (i = 0; i < IO_MAX; i++)
>> +    {
>> +        addr = acpi_mmio_region[i][0];
>> +        size = acpi_mmio_region[i][1];
>>
>> +        res = iomem_permit_access(d, paddr_to_pfn(addr & PAGE_MASK),
>> +                                  paddr_to_pfn(PAGE_ALIGN(addr + size - 
>> 1)));
>> +        res = map_mmio_regions(d,
>> +                               paddr_to_pfn(addr & PAGE_MASK),
>> +                               DIV_ROUND_UP(size, PAGE_SIZE),
>> +                               paddr_to_pfn(addr & PAGE_MASK));
>> +
>> +
>> +    }
>> +#if 0
>> +     for( i=32 ; i < 255 ; i++ )
>> +     {
>> +        res = vgic_reserve_virq(d, i);
>> +        res = route_irq_to_guest(d, i, NULL);
>> +         if ( res )
>> +        {
>> +            printk(XENLOG_ERR "Unable to route IRQ %u to domain %u\n",
>> +                   i, d->domain_id);
>> +        }
>> +
>> +     }
>> +#endif
>> +     return 0;
>> +}
>> +#else
>>  static int acpi_map_mmio(struct domain *d)
>>  {
>>      int i,res;
>> @@ -1273,7 +1353,7 @@ static int acpi_map_mmio(struct domain *d)
>>
>>       return 0;
>>  }
>> -
>> +#endif
>
> This is pretty terrible. You are ifdef'ing out the entire function that
> you introduced earlier.
i will refactor it to make it more clear, its essentially same
function but is reading mmio regions locally instead of uefi tables.
>
>>  static int map_acpi_regions(struct domain *d)
>>  {
>>      int res;
>> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
>> index 97061ce..e74555d 100644
>> --- a/xen/arch/arm/vgic.c
>> +++ b/xen/arch/arm/vgic.c
>> @@ -254,6 +254,8 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
>>      }
>>  }
>>
>> +#define VGIC_ICFG_MASK(intr) ( 1 << ( ( 2 * ( intr % 16 ) ) + 1 ) )
>> +
>>  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>>  {
>>      struct domain *d = v->domain;
>> @@ -266,6 +268,20 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>>
>>      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>>          irq = i + (32 * n);
>> +#if defined(CONFIG_ARM_64) && defined(CONFIG_ACPI)
>> +        if( ( n!=0 ) && is_hardware_domain(d) ){
>> +            struct vgic_irq_rank *vr = vgic_get_rank(v, n);
>> +            uint32_t tr;
>> +            tr = vr->icfg[i >> 4] ;
>> +
>> +            if( ( tr & VGIC_ICFG_MASK(i) ) )
>> +                acpi_set_irq(irq, DT_IRQ_TYPE_EDGE_BOTH);
>> +            else
>> +                acpi_set_irq(irq, DT_IRQ_TYPE_LEVEL_MASK);
>> +
>> +            route_irq_to_guest(d,irq,NULL);
>> +        }
>> +#endif
>
> Is this the code that currently assigns all the irqs to dom0?
yes
>
>>          v_target = d->arch.vgic.handler->get_target_vcpu(v, irq);
>>          p = irq_to_pending(v_target, irq);
>>          set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
>> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
>> index 73da9d9..2d78ba0 100644
>> --- a/xen/drivers/acpi/osl.c
>> +++ b/xen/drivers/acpi/osl.c
>> @@ -66,7 +66,7 @@ void __init acpi_os_vprintf(const char *fmt, va_list args)
>>
>>  acpi_physical_address __init acpi_os_get_root_pointer(void)
>>  {
>> -     if (efi_enabled) {
>> +    if (efi_enabled) {
>>               if (efi.acpi20 != EFI_INVALID_TABLE_ADDR)
>>                       return efi.acpi20;
>>               else if (efi.acpi != EFI_INVALID_TABLE_ADDR)
>> @@ -198,8 +198,11 @@ acpi_os_write_memory(acpi_physical_address phys_addr, 
>> u32 value, u32 width)
>>
>>       return AE_OK;
>>  }
>> -
>> +#ifdef CONFIG_X86
>>  #define is_xmalloc_memory(ptr) ((unsigned long)(ptr) & (PAGE_SIZE - 1))
>> +#else
>> +#define is_xmalloc_memory(ptr) 1
>> +#endif
>
> Why?
this test is failing,leading to crash.
>
>>  void *__init acpi_os_alloc_memory(size_t sz)
>>  {
>> --
>> 1.9.1
>>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.