Hi Robin, On 1/14/19 12:10 PM, Robin Murphy wrote: > On 10/01/2019 10:44, Auger Eric wrote: >> Hi Robin, Drew, >> >> On 12/19/18 2:18 PM, Andrew Jones wrote: >>> On Wed, Dec 19, 2018 at 12:21:35PM +0000, Robin Murphy wrote: >>>> On 18/12/2018 18:48, Andrew Jones wrote: >>>>> The sum of dmaaddr and size may overflow, particularly considering >>>>> there are cases where size will be U64_MAX. >>>> >>>> Only if the firmware is broken in the first place, though. It would >>>> be weird >>>> to describe an explicit _DMA range of base=0 and size=U64_MAX, >>>> because it's >>>> effectively the same as just not having one at all, but it's not >>>> strictly >>>> illegal. However, since the ACPI System Memory address space is at most >>>> 64-bit, anything that would actually overflow here is already >>>> describing an >>>> impossibility - really, we should probably scream even louder about a >>>> firmware bug and reject it entirely, rather than quietly hiding it. >>> >>> Ah, looking again I see the paths. Either acpi_dma_get_range() returns >>> success, in which case base and size are fine, or it returns an EINVAL, >>> in which case base=size=0, or it returns ENODEV in which case base is >>> zero, so size may be set to U64_MAX by rc_dma_get_range() with no >>> problem. >>> The !dev_is_pci(dev) path is also fine since base=0. >> >> So practically putting an explicit memory_address_limit=64 is harmless >> as dmaaddr always is 0, right? >> >> In QEMU I intended to update the ACPI code to comply with the rev D >> spec. in that case the RC table revision is 1 (rev D) and the >> memory_address_limit needs to be filled. If we don't want to restrict >> the limit, isn't it the right choice to set 64 here? > > Indeed, the Memory Address Size Limit doesn't cater for offsets so can't > run into this kind of overflow in the first place. For a fully-emulated > PCI hierarchy I'd say 64 is not just harmless but in fact entirely > correct - you're going to have more fun with VFIO passthrough if the > host tables have more restrictive limits, but I guess that's a problem > for the future ;) Yes but that's a good point to notice! Thanks Eric > > Robin.
