Re: [Xen-devel] [PATCH v5 1/1] Add mmio_hole_size

[George Dunlap <dunlapg@xxxxxxxxx>]

On Tue, Sep 30, 2014 at 2:27 AM, Boris Ostrovsky
<boris.ostrovsky@xxxxxxxxxx> wrote:
>> +    if ( mmio_hole_size )
>> +    {
>> +        uint64_t max_ram_below_4g = (1ULL << 32) - mmio_hole_size;
>> +
>> +        if ( max_ram_below_4g > HVM_BELOW_4G_MMIO_START )
>> +        {
>> +            printf("max_ram_below_4g=0x"PRIllx
>> +                   " too big for mmio_hole_size=0x"PRIllx
>> +                   " has been ignored.\n",
>> +                   PRIllx_arg(max_ram_below_4g),
>> +                   PRIllx_arg(mmio_hole_size));
>> +        }
>
>
> Do you need to check whether the hole is too large?
>
> Here and in the toolstack.

How large is too large?  I've seen real machines with a 3GiB memory hole...

>> "0";
>> +        if (info->u.hvm.mmio_hole_size) {
>> +            uint64_t max_ram_below_4g =
>> +                (1ULL << 32) - info->u.hvm.mmio_hole_size;
>> +
>> +            if (max_ram_below_4g <= HVM_BELOW_4G_MMIO_START) {
>
>
> You seem to be making various checks on hole size in multiple places ---
> here, in libxl__build_device_model_args_new() and in parse_config_data().
> (And, to some degree, in xc_hvm_build_with_hole()). Are they all necessary?

I think we should get rid of xc_hvm_build_with_hole() entirely, but I
think we want a check everywhere else.  The key question is when the
failure is going to happen ideally.  You absolutely have to check it
at the bottom level (hvmloader).  But you don't want to go through all
the hassle of setting up the domain and starting to boot it only to
find that some silly parameter is messed up, so it should also be
checked in libxl (since many people will be going through only libxl).
But if you're using xl and get an error at the libxl layer, the error
message is often a bit cryptic.  So it makes sense to have it at the
parsing level too.

 -George

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