[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xen/pass-through: ROM BAR handling adjustments
>>> On 05.06.15 at 18:41, <stefano.stabellini@xxxxxxxxxxxxx> wrote: > On Fri, 5 Jun 2015, Jan Beulich wrote: >> >>> On 05.06.15 at 13:32, <stefano.stabellini@xxxxxxxxxxxxx> wrote: >> >> --- a/hw/xen/xen_pt.c >> >> +++ b/hw/xen/xen_pt.c >> >> @@ -248,7 +248,9 @@ static void xen_pt_pci_write_config(PCID >> >> >> >> /* check unused BAR register */ >> >> index = xen_pt_bar_offset_to_index(addr); >> >> - if ((index >= 0) && (val > 0 && val < XEN_PT_BAR_ALLF) && >> >> + if ((index >= 0) && (val != 0) && >> >> + (((index != PCI_ROM_SLOT) ? >> >> + val : (val | (uint32_t)~PCI_ROM_ADDRESS_MASK)) != > XEN_PT_BAR_ALLF) && >> > >> > The change seems looks good and in line with the commit message. But >> > this if statement looks like acrobatic circus to me now. >> >> I think the alternative of splitting it up into multiple if()-s would not >> be any better readable. > > Would you be OK if I rewrote the statement as follows? > > if ((index >= 0) && (val != 0)) { > uint32_t vu; > > if (index == PCI_ROM_SLOT) { > vu = val | (uint32_t)~PCI_ROM_ADDRESS_MASK; > } else { > vu = val; > } > > if ((vu != XEN_PT_BAR_ALLF) && > (s->bases[index].bar_flag == XEN_PT_BAR_FLAG_UNUSED)) { > XEN_PT_WARN(d, "Guest attempt to set address to unused Base > Address " > "Register. (addr: 0x%02x, len: %d)\n", addr, len); > } > } Actually I agree that this indeed looks better. If I were to make the change, I'd do if ((index >= 0) && (val != 0)) { uint32_t vu = val; if (index == PCI_ROM_SLOT) { vu |= (uint32_t)~PCI_ROM_ADDRESS_MASK; } if ((vu != XEN_PT_BAR_ALLF) && ... though. But if you're going to do the edit without wanting me to re-submit, I'll certainly leave this to you. Just let me know which way you prefer it to be handled. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |