From mboxrd@z Thu Jan 1 00:00:00 1970 From: Malcolm Crossley Subject: Re: [PATCH] hw/passthrough: Prevent QEMU from mapping PCI option ROM at address 0 Date: Mon, 12 May 2014 14:26:58 +0100 Message-ID: <5370CC22.6020408@citrix.com> References: <1399898571-6011-1-git-send-email-malcolm.crossley@citrix.com> <5370E44102000078000116AD@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5370E44102000078000116AD@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: ian.jackson@eu.citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 12/05/14 14:09, Jan Beulich wrote: >>>> On 12.05.14 at 14:42, 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 non address parts of the BAR before comparing >> the address to 0. >> --- >> 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..7d6aefc 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_BASE_ADDRESS_MEM_MASK) == 0)) > > You talk of the low bit, but mask off the low 4 - how does that fit > together? Didn't you rather mean PCI_ROM_ADDRESS_MASK & > ~PCI_ROM_ADDRESS_ENABLE in text and code? The description provides an example of a driver setting the lower bits of the BAR. The intent of the fix is to ensure no BAR is mapped address 0 which is achieved by ensuring only the address bits of the BAR are used for the comparison with 0. Malcolm > Jan >