Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [Xen-devel] [PATCH v4] xen-pciback: optionally allow interrupt
 enable flag writes
To:     =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= <roger.pau@citrix.com>,
        =?UTF-8?Q?Marek_Marczykowski-G=c3=b3recki?= 
        <marmarek@invisiblethingslab.com>
Cc:     xen-devel@lists.xenproject.org, Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        YueHaibing <yuehaibing@huawei.com>,
        open list <linux-kernel@vger.kernel.org>,
        Simon Gaiser <simon@invisiblethingslab.com>,
        Jan Beulich <jbeulich@suse.com>
References: <20200115014643.12749-1-marmarek@invisiblethingslab.com>
 <20200115164815.GO11756@Air-de-Roger>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
Message-ID: <393ff73f-802c-9f1c-b739-4476388b6c98@oracle.com>
Date:   Wed, 15 Jan 2020 17:32:32 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.3.0
MIME-Version: 1.0
In-Reply-To: <20200115164815.GO11756@Air-de-Roger>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 1/15/20 11:48 AM, Roger Pau Monné wrote:
> On Wed, Jan 15, 2020 at 02:46:29AM +0100, Marek Marczykowski-Górecki wrote:
>> QEMU running in a stubdom needs to be able to set INTX_DISABLE, and the
>> MSI(-X) enable flags in the PCI config space. This adds an attribute
>> 'allow_interrupt_control' which when set for a PCI device allows writes
>> to this flag(s). The toolstack will need to set this for stubdoms.
>> When enabled, guest (stubdomain) will be allowed to set relevant enable
>> flags, but only one at a time - i.e. it refuses to enable more than one
>> of INTx, MSI, MSI-X at a time.
>>
>> This functionality is needed only for config space access done by device
>> model (stubdomain) serving a HVM with the actual PCI device. It is not
>> necessary and unsafe to enable direct access to those bits for PV domain
>> with the device attached. For PV domains, there are separate protocol
>> messages (XEN_PCI_OP_{enable,disable}_{msi,msix}) for this purpose.
>> Those ops in addition to setting enable bits, also configure MSI(-X) in
>> dom0 kernel - which is undesirable for PCI passthrough to HVM guests.
>>
>> This should not introduce any new security issues since a malicious
>> guest (or stubdom) can already generate MSIs through other ways, see
>> [1] page 8. Additionally, when qemu runs in dom0, it already have direct
>> access to those bits.
>>
>> This is the second iteration of this feature. First was proposed as a
>> direct Xen interface through a new hypercall, but ultimately it was
>> rejected by the maintainer, because of mixing pciback and hypercalls for
>> PCI config space access isn't a good design. Full discussion at [2].
>>
>> [1]: https://invisiblethingslab.com/resources/2011/Software%20Attacks%20on%20Intel%20VT-d.pdf
>> [2]: https://xen.markmail.org/thread/smpgpws4umdzizze
>>
>> [part of the commit message and sysfs handling]
>> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
>> [the rest]
>> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Some minor nits below, but LGTM:
>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-pciback b/Documentation/ABI/testing/sysfs-driver-pciback
>> index 6a733bfa37e6..566a11f2c12f 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-pciback
>> +++ b/Documentation/ABI/testing/sysfs-driver-pciback
>> @@ -11,3 +11,16 @@ Description:
>>                   #echo 00:19.0-E0:2:FF > /sys/bus/pci/drivers/pciback/quirks
>>                   will allow the guest to read and write to the configuration
>>                   register 0x0E.
>> +
>> +What:           /sys/bus/pci/drivers/pciback/allow_interrupt_control
>> +Date:           Jan 2020
>> +KernelVersion:  5.5

5.6

I can fix this and the things that Roger mentioned while committing if 
Marek is OK with that.

-boris