[PATCH v3] hw/passthrough: Prevent QEMU from mapping PCI option ROM at address 0

[PATCH v3] hw/passthrough: Prevent QEMU from mapping PCI option ROM at address 0

[Malcolm Crossley]

The PCI option ROM BAR uses the LSB to indicate if the BAR is enabled.
The AMD graphics driver sets the address bit's of the BAR to 0 but leaves the
LSB set to 1. Whilst this is not good practice, QEMU should be ignoring the
non address parts of the BAR.

This patch adds masking of the bits 0-10 (4k page) parts of the BAR
before comparing the address to 0.

Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
---
 hw/pass-through.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/pass-through.c b/hw/pass-through.c
index 304c438..f83c88c 100644
--- a/hw/pass-through.c
+++ b/hw/pass-through.c
@@ -2208,7 +2208,7 @@ static void pt_bar_mapping_one(struct pt_dev *ptdev, int bar, int io_enable,
     }
 
     /* prevent guest software mapping memory resource to 00000000h */
-    if ((base->bar_flag == PT_BAR_FLAG_MEM) && (r_addr == 0))
+    if ((base->bar_flag == PT_BAR_FLAG_MEM) && ((r_addr & PCI_ROM_ADDRESS_MASK) == 0))
         r_addr = -1;
 
     /* align resource size (memory type only) */
-- 
1.7.1

Re: [PATCH v3] hw/passthrough: Prevent QEMU from mapping PCI option ROM at address 0

[Jan Beulich]

>>> On 12.05.14 at 16:37, <malcolm.crossley@citrix.com> wrote:
> The PCI option ROM BAR uses the LSB to indicate if the BAR is enabled.
> The AMD graphics driver sets the address bit's of the BAR to 0 but leaves the
> LSB set to 1. Whilst this is not good practice, QEMU should be ignoring the
> non address parts of the BAR.
> 
> This patch adds masking of the bits 0-10 (4k page) parts of the BAR
> before comparing the address to 0.

Sorry, but to avoid confusing (future) readers - this is 2k units, not
4k pages.

> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>

With the description corrected:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH v3] hw/passthrough: Prevent QEMU from mapping PCI option ROM at address 0

[Malcolm Crossley]

On 12/05/14 15:53, Jan Beulich wrote:
>>>> On 12.05.14 at 16:37, <malcolm.crossley@citrix.com> wrote:
>> The PCI option ROM BAR uses the LSB to indicate if the BAR is enabled.
>> The AMD graphics driver sets the address bit's of the BAR to 0 but leaves the
>> LSB set to 1. Whilst this is not good practice, QEMU should be ignoring the
>> non address parts of the BAR.
>>
>> This patch adds masking of the bits 0-10 (4k page) parts of the BAR
>> before comparing the address to 0.
> 
> Sorry, but to avoid confusing (future) readers - this is 2k units, not
> 4k pages.
> 
Thanks for pointing that out Jan, I was actually intending to ensure
nothing is allowed to be mapped to the 0 page and did not realise the
option ROM BAR is 2k aligned instead of 4k. I will submit a new patch
using XC_PAGE_MASK instead.

>> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
> 
> With the description corrected:
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
>