Re: [PATCH 02/10] KVM: s390: add the GIB and its related life-cyle functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/11/2018 17:23, Michael Mueller wrote:


On 05.11.18 16:44, Janosch Frank wrote:
On 05.11.18 16:36, Halil Pasic wrote:

On 11/05/2018 04:14 PM, Michael Mueller wrote:

On 05.11.18 15:51, Pierre Morel wrote:
On 25/10/2018 14:37, Michael Mueller wrote:
[..]
+
+void kvm_s390_gib_init(u8 nisc)
+{
+    int rc;
+
+    if (gib)
+        return;
+
+    if (!css_general_characteristics.aiv) {
+        KVM_EVENT(3, "%s", "gib not initialized, no AIV facility");
+        return;
+    }

Here we make sure we have the AIV facility...
This is the only recoverable error in the initialization.

+
+    gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL | GFP_DMA);
+    if (!gib) {
+        KVM_EVENT(3, "%s", "gib memory allocation failed");
+        return;
+    }
Why should we survive this error?
Design question.

I'm with Pierre on this one. We should fail starting the guest
if this allocation fails.

Halil, the GIB is global to KVM, we would need to fail loading/init of
KVM, which I'd like to avoid.

Neither GIB nor gisa is necessary for a VM to run, so we shouldn't stop
on an error.

That is my point as well!

two errors two point of view:

On this error, the problem is that it is an allocation error done during the loading of a module.

Why should we go on on such an error ?
Question is independent of the GIB!




+
+    gib->nisc = nisc;
+    rc = chsc_sgib((u32)(u64)gib);
+    if (rc) {
+        KVM_EVENT(3, "gib 0x%pK AIV association failed rc: %d",
+              gib, rc);
+        free_page((unsigned long)gib);
+        gib = NULL;
+        return;
+    }
or this failure ?

shouldn't we better return an error and abort loading KVM?
Design question.

Same here I guess. This would be only either due to a guest or
a host bug, or?

I would prefer being fairly deterministic about whether we do have
a gib or not.
Same as above

For this error, we should decide if we use GIB or not.
If it is a configuration problem let us treat it as a configuration issue and use a CPU feature.

If we decide to do it undercover, we can *not* let the user use GISA without GIB. For what I understood GIB is always available if AIV, so we have our feature, if it is available this error should never happen.
If it does it is a system failure.
We should not continue with a broken system.


Regards,
Halil





--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux