Re: [Xen-devel] [PATCH RFC] ioreq-server: handle the lack of a default emulator properly

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.09.14 at 10:59, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 26 September 2014 17:23
>> To: Paul Durrant
>> Cc: xen-devel@xxxxxxxxxxxxx; Keir (Xen.org)
>> Subject: Re: [PATCH RFC] ioreq-server: handle the lack of a default emulator
>> properly
>> 
>> >>> On 26.09.14 at 17:31, <paul.durrant@xxxxxxxxxx> wrote:
>> > This fix should probably go in 4.5 but this patch is RFC for the moment
>> > because of uncertainty about what to do for unemulated MMIO accesses.
>> > Originally I forced a domain crash in this case, but hvmloader actually
>> > hit the crash because the code that deals with building the ACPI TPM
>> > info reads from 0xFED40F00 looking for a signature value and there is
>> > nothing backing this access in my configuration. So, the question is
>> > whether to whitelist this access in some way or make building that
>> > table optional in some way so that it is only invoked if an emulated
>> > TPM is definitely present.
>> 
>> I think in the sense of acting like real hardware, having a definite
>> point to complete such I/O is quite desirable, as opposed to killing
>> the guest.
> 
> Ok. The doubt was around the fact that on bare metal an unhandled MMIO would 
> normally cause an MCE (by way of a bus error).

Hmm, no, I don't think so, particularly on x86: Together with the
chipset, all transactions should have an instance completing them.
And I don't think MCEs would be the way to signal failure thereof.
All this may be different on other architectures, but x86's heritage
is that you can access any "unused" memory location without
causing the machine to lock up (other than e.g. on ia64).

>> > --- a/xen/arch/x86/hvm/hvm.c
>> > +++ b/xen/arch/x86/hvm/hvm.c
>> > @@ -2386,8 +2386,7 @@ static struct hvm_ioreq_server
>> *hvm_select_ioreq_server(struct domain *d,
>> >      if ( list_empty(&d->arch.hvm_domain.ioreq_server.list) )
>> >          return NULL;
>> >
>> > -    if ( list_is_singular(&d->arch.hvm_domain.ioreq_server.list) ||
>> > -         (p->type != IOREQ_TYPE_COPY && p->type != IOREQ_TYPE_PIO) )
>> > +    if ( p->type != IOREQ_TYPE_COPY && p->type != IOREQ_TYPE_PIO )
>> 
>> I'm having some trouble understanding the reason for this change.
>> 
> 
> The implication that list_is_singular -> 'singular ioreq server is default' 
> is 
> wrong; it was an 'optimization' too far. Now that I have QEMU ported, in my 
> tests it is the only emulator but it is not a default emulator (because it 
> registers explicit ranges).

Ah, that makes sense of course. In which case I'm fine with the
patch as is.

Jan


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