Re: [Xen-devel] [PATCH] xen/pass-through: ROM BAR handling adjustments

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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);
        }
    }


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel