lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <422b6060-3c58-0bd3-b0ee-78803b5da7fc@redhat.com>
Date:   Mon, 17 Oct 2016 16:25:36 +0200
From:   Auger Eric <eric.auger@...hat.com>
To:     Punit Agrawal <punit.agrawal@....com>
Cc:     yehuday@...vell.com, drjones@...hat.com, jason@...edaemon.net,
        kvm@...r.kernel.org, marc.zyngier@....com, p.fedin@...sung.com,
        joro@...tes.org, will.deacon@....com, linux-kernel@...r.kernel.org,
        iommu@...ts.linux-foundation.org, alex.williamson@...hat.com,
        pranav.sawargaonkar@...il.com,
        linux-arm-kernel@...ts.infradead.org, tglx@...utronix.de,
        robin.murphy@....com, Manish.Jaggi@...iumnetworks.com,
        christoffer.dall@...aro.org, eric.auger.pro@...il.com
Subject: Re: [PATCH v14 04/16] iommu/dma: MSI doorbell alloc/free

Hi Punit,

On 14/10/2016 13:25, Punit Agrawal wrote:
> Hi Eric,
> 
> One query and a comment below.
> 
> Eric Auger <eric.auger@...hat.com> writes:
> 
>> We introduce the capability to (un)register MSI doorbells.
>>
>> A doorbell region is characterized by its physical address base, size,
>> and whether it its safe (ie. it implements IRQ remapping). A doorbell
>> can be per-cpu or global. We currently only care about global doorbells.
>>
>> A function returns whether all registered doorbells are safe.
>>
>> MSI controllers likely to work along with IOMMU that translate MSI
>> transaction must register their doorbells to allow device assignment
>> with MSI support.  Otherwise the MSI transactions will cause IOMMU faults.
>>
>> Signed-off-by: Eric Auger <eric.auger@...hat.com>
>>
>> ---
>>
>> v13 -> v14:
>> - previously in msi-doorbell.h/c
>> ---
>>  drivers/iommu/dma-iommu.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/dma-iommu.h | 41 ++++++++++++++++++++++++++
>>  2 files changed, 116 insertions(+)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index d45f9a0..d8a7d86 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -43,6 +43,38 @@ struct iommu_dma_cookie {
>>  	spinlock_t		msi_lock;
>>  };
>>  
>> +/**
>> + * struct iommu_msi_doorbell_info - MSI doorbell region descriptor
>> + * @percpu_doorbells: per cpu doorbell base address
>> + * @global_doorbell: base address of the doorbell
>> + * @doorbell_is_percpu: is the doorbell per cpu or global?
>> + * @safe: true if irq remapping is implemented
>> + * @size: size of the doorbell
>> + */
>> +struct iommu_msi_doorbell_info {
>> +	union {
>> +		phys_addr_t __percpu    *percpu_doorbells;
> 
> Out of curiosity, have you come across systems that have per-cpu
> doorbells? I couldn't find a system that'd help solidify my
> understanding on it's usage.

This came out after a discussion With Marc. However at the moment I am
not aware of any MSI controller featuring per-cpu doorbell. Not sure
whether it stays relevant to keep this notion at that stage.

> 
>> +		phys_addr_t             global_doorbell;
>> +	};
>> +	bool    doorbell_is_percpu;
>> +	bool    safe;
> 
> Although you've got the comment above, 'safe' doesn't quite convey it's
> purpose. Can this be renamed to something more descriptive -
> 'intr_remapping' or 'intr_isolation' perhaps?

Yes definitively

Thanks

Eric
> 
> Thanks,
> Punit
> 
> 
> [...]
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ